diff --git a/Documentation/rest-api-groups.txt b/Documentation/rest-api-groups.txt index 486a826182..f4f16c226c 100644 --- a/Documentation/rest-api-groups.txt +++ b/Documentation/rest-api-groups.txt @@ -638,6 +638,9 @@ get::/groups/1/members/ To resolve the included groups of a group recursively and to list all members the parameter `recursive` can be set. +Members from included external groups and from included groups which +are not visible to the calling user are ignored. + .Request ---- GET /groups/834ec36dd5e0ed21a2ff5d7e2255da082d63bbd7/members/?recursive HTTP/1.0 diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GetGroups.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GetGroups.java index d2706566f7..97e4e70325 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GetGroups.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GetGroups.java @@ -16,7 +16,6 @@ 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; @@ -39,8 +38,7 @@ public class GetGroups implements RestReadView { } @Override - public List apply(AccountResource resource) - throws ResourceNotFoundException, OrmException { + public List apply(AccountResource resource) throws OrmException { IdentifiedUser user = resource.getUser(); Account.Id userId = user.getAccountId(); List groups = Lists.newArrayList(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/AddIncludedGroups.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/AddIncludedGroups.java index 79b80e5fc6..ae9a874d8c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/AddIncludedGroups.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/AddIncludedGroups.java @@ -82,8 +82,8 @@ public class AddIncludedGroups implements RestModifyView { @Override public List apply(GroupResource resource, Input input) - throws ResourceNotFoundException, MethodNotAllowedException, - AuthException, BadRequestException, OrmException { + throws MethodNotAllowedException, AuthException, BadRequestException, + OrmException { AccountGroup group = resource.toAccountGroup(); if (group == null) { throw new MethodNotAllowedException(); @@ -153,8 +153,8 @@ public class AddIncludedGroups implements RestModifyView { @Override public GroupInfo apply(GroupResource resource, Input input) - throws ResourceNotFoundException, MethodNotAllowedException, - AuthException, BadRequestException, OrmException { + throws MethodNotAllowedException, AuthException, BadRequestException, + OrmException { AddIncludedGroups.Input in = new AddIncludedGroups.Input(); in.groups = ImmutableList.of(id); List list = put.get().apply(resource, in); @@ -178,8 +178,7 @@ public class AddIncludedGroups implements RestModifyView { @Override public Object apply(IncludedGroupResource resource, - PutIncludedGroup.Input input) throws ResourceNotFoundException, - MethodNotAllowedException, OrmException { + PutIncludedGroup.Input input) throws MethodNotAllowedException, OrmException { // Do nothing, the group is already included. return get.get().apply(resource); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/CreateGroup.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/CreateGroup.java index d78c7df3c1..e2b972b709 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/CreateGroup.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/CreateGroup.java @@ -78,7 +78,7 @@ class CreateGroup implements RestModifyView { @Override public GroupInfo apply(TopLevelResource resource, Input input) - throws ResourceNotFoundException, AuthException, BadRequestException, + throws AuthException, BadRequestException, NameAlreadyUsedException, OrmException { if (input == null) { input = new Input(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/GetDetail.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/GetDetail.java index adb04cdb77..f1d2e15fa6 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/GetDetail.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/GetDetail.java @@ -15,7 +15,6 @@ 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; @@ -31,8 +30,7 @@ public class GetDetail implements RestReadView { } @Override - public GroupInfo apply(GroupResource rsrc) throws ResourceNotFoundException, - OrmException { + public GroupInfo apply(GroupResource rsrc) throws OrmException { return json.format(rsrc); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/GetGroup.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/GetGroup.java index 32cf9be438..09180dd6a1 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/GetGroup.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/GetGroup.java @@ -14,7 +14,6 @@ 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; @@ -29,8 +28,7 @@ class GetGroup implements RestReadView { } @Override - public GroupInfo apply(GroupResource resource) - throws ResourceNotFoundException, OrmException { + public GroupInfo apply(GroupResource resource) throws OrmException { return json.format(resource.getGroup()); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/GetIncludedGroup.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/GetIncludedGroup.java index 7e63816f09..32f20c0c2d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/GetIncludedGroup.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/GetIncludedGroup.java @@ -14,7 +14,6 @@ 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; @@ -29,8 +28,7 @@ public class GetIncludedGroup implements RestReadView { } @Override - public GroupInfo apply(IncludedGroupResource rsrc) - throws ResourceNotFoundException, OrmException { + public GroupInfo apply(IncludedGroupResource rsrc) throws OrmException { return json.format(rsrc.getMemberDescription()); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/GroupJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/GroupJson.java index cc57b3c684..74ae03c5fe 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/GroupJson.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/GroupJson.java @@ -22,7 +22,6 @@ 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.MethodNotAllowedException; -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; @@ -65,15 +64,13 @@ public class GroupJson { return this; } - public GroupInfo format(GroupResource rsrc) throws ResourceNotFoundException, - OrmException { + public GroupInfo format(GroupResource rsrc) throws OrmException { GroupInfo info = init(rsrc.getGroup()); initMembersAndIncludes(rsrc, info); return info; } - public GroupInfo format(GroupDescription.Basic group) - throws ResourceNotFoundException, OrmException { + public GroupInfo format(GroupDescription.Basic group) throws OrmException { GroupInfo info = init(group); if (options.contains(MEMBERS) || options.contains(INCLUDES)) { GroupResource rsrc = @@ -107,7 +104,7 @@ public class GroupJson { } private GroupInfo initMembersAndIncludes(GroupResource rsrc, GroupInfo info) - throws ResourceNotFoundException, OrmException { + throws OrmException { if (rsrc.toAccountGroup() == null) { return info; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/ListGroups.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/ListGroups.java index 88a7ca8df8..d43ae69d88 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/ListGroups.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/ListGroups.java @@ -187,7 +187,7 @@ public class ListGroups implements RestReadView { } private List getGroupsOwnedBy(IdentifiedUser user) - throws ResourceNotFoundException, OrmException { + throws OrmException { List groups = Lists.newArrayList(); for (AccountGroup g : filterGroups(groupCache.all())) { GroupControl ctl = groupControlFactory.controlFor(g); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/ListIncludedGroups.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/ListIncludedGroups.java index 31332d3827..3691e805ec 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/ListIncludedGroups.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/ListIncludedGroups.java @@ -19,7 +19,6 @@ import static com.google.common.base.Strings.nullToEmpty; import com.google.common.collect.Lists; import com.google.gerrit.common.errors.NoSuchGroupException; import com.google.gerrit.extensions.restapi.MethodNotAllowedException; -import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.RestReadView; import com.google.gerrit.reviewdb.client.AccountGroupIncludeByUuid; import com.google.gerrit.reviewdb.server.ReviewDb; @@ -52,7 +51,7 @@ public class ListIncludedGroups implements RestReadView { @Override public List apply(GroupResource rsrc) - throws MethodNotAllowedException, ResourceNotFoundException, OrmException { + throws MethodNotAllowedException, OrmException { if (rsrc.toAccountGroup() == null) { throw new MethodNotAllowedException(); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/ListMembers.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/ListMembers.java index a9aae17794..d32c632233 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/ListMembers.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/ListMembers.java @@ -21,7 +21,6 @@ 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.MethodNotAllowedException; -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; @@ -60,33 +59,28 @@ public class ListMembers implements RestReadView { @Override public List apply(final GroupResource resource) - throws MethodNotAllowedException, ResourceNotFoundException, OrmException { + throws MethodNotAllowedException, OrmException { if (resource.toAccountGroup() == null) { throw new MethodNotAllowedException(); } - try { - final Map members = - getMembers(resource.getGroupUUID(), new HashSet()); - final List memberInfos = Lists.newArrayList(members.values()); - Collections.sort(memberInfos, new Comparator() { - @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()); - } + final Map members = + getMembers(resource.getGroupUUID(), new HashSet()); + final List memberInfos = Lists.newArrayList(members.values()); + Collections.sort(memberInfos, new Comparator() { + @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; } private Map getMembers( final AccountGroup.UUID groupUUID, - final HashSet seenGroups) throws OrmException, - NoSuchGroupException { + final HashSet seenGroups) throws OrmException { seenGroups.add(groupUUID); final Map members = Maps.newHashMap(); @@ -96,8 +90,13 @@ public class ListMembers implements RestReadView { return Collections.emptyMap(); } - final GroupDetail groupDetail = - groupDetailFactory.create(group.getId()).call(); + final GroupDetail groupDetail; + try { + groupDetail = groupDetailFactory.create(group.getId()).call(); + } catch (NoSuchGroupException e) { + // the included group is not visible + return Collections.emptyMap(); + } if (groupDetail.members != null) { for (final AccountGroupMember m : groupDetail.members) {