opendev/gerrit
Code Issues Proposed changes

Merge "Ignore non-visible included groups when listing members recursively"

Browse Source
This commit is contained in:
Shawn Pearce
2013-03-11 14:51:33 +00:00
committed by Gerrit Code Review
parent 9b5fc765db d54de1cbb6
commit f673a97d7c
11 changed files with 40 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
Documentation/rest-api-groups.txt
View File

@@ -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

4
gerrit-server/src/main/java/com/google/gerrit/server/account/GetGroups.java
View File

@@ -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<AccountResource> {
}
@Override
public List<GroupInfo> apply(AccountResource resource)
throws ResourceNotFoundException, OrmException {
public List<GroupInfo> apply(AccountResource resource) throws OrmException {
IdentifiedUser user = resource.getUser();
Account.Id userId = user.getAccountId();
List<GroupInfo> groups = Lists.newArrayList();

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

@@ -82,8 +82,8 @@ public class AddIncludedGroups implements RestModifyView<GroupResource, Input> {
@Override
public List<GroupInfo> 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<GroupResource, Input> {
@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<GroupInfo> list = put.get().apply(resource, in);
@@ -178,8 +178,7 @@ public class AddIncludedGroups implements RestModifyView<GroupResource, Input> {
@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);
}

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

@@ -78,7 +78,7 @@ class CreateGroup implements RestModifyView<TopLevelResource, Input> {
@Override
public GroupInfo apply(TopLevelResource resource, Input input)
throws ResourceNotFoundException, AuthException, BadRequestException,
throws AuthException, BadRequestException,
NameAlreadyUsedException, OrmException {
if (input == null) {
input = new Input();

4
gerrit-server/src/main/java/com/google/gerrit/server/group/GetDetail.java
View File

@@ -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<GroupResource> {
}
@Override
public GroupInfo apply(GroupResource rsrc) throws ResourceNotFoundException,
OrmException {
public GroupInfo apply(GroupResource rsrc) throws OrmException {
return json.format(rsrc);
}
}

4
gerrit-server/src/main/java/com/google/gerrit/server/group/GetGroup.java
View File

@@ -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<GroupResource> {
}
@Override
public GroupInfo apply(GroupResource resource)
throws ResourceNotFoundException, OrmException {
public GroupInfo apply(GroupResource resource) throws OrmException {
return json.format(resource.getGroup());
}
}

4
gerrit-server/src/main/java/com/google/gerrit/server/group/GetIncludedGroup.java
View File

@@ -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<IncludedGroupResource> {
}
@Override
public GroupInfo apply(IncludedGroupResource rsrc)
throws ResourceNotFoundException, OrmException {
public GroupInfo apply(IncludedGroupResource rsrc) throws OrmException {
return json.format(rsrc.getMemberDescription());
}
}

9
gerrit-server/src/main/java/com/google/gerrit/server/group/GroupJson.java
View File

@@ -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;
}

2
gerrit-server/src/main/java/com/google/gerrit/server/group/ListGroups.java
View File

@@ -187,7 +187,7 @@ public class ListGroups implements RestReadView<TopLevelResource> {
}
private List<GroupInfo> getGroupsOwnedBy(IdentifiedUser user)
throws ResourceNotFoundException, OrmException {
throws OrmException {
List<GroupInfo> groups = Lists.newArrayList();
for (AccountGroup g : filterGroups(groupCache.all())) {
GroupControl ctl = groupControlFactory.controlFor(g);

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

@@ -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<GroupResource> {
@Override
public List<GroupInfo> apply(GroupResource rsrc)
throws MethodNotAllowedException, ResourceNotFoundException, OrmException {
throws MethodNotAllowedException, OrmException {
if (rsrc.toAccountGroup() == null) {
throw new MethodNotAllowedException();
}

45
gerrit-server/src/main/java/com/google/gerrit/server/group/ListMembers.java
View File

@@ -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<GroupResource> {
@Override
public List<AccountInfo> apply(final GroupResource resource)
throws MethodNotAllowedException, ResourceNotFoundException, OrmException {
throws MethodNotAllowedException, OrmException {
if (resource.toAccountGroup() == null) {
throw new MethodNotAllowedException();
}
try {
final Map<Account.Id, AccountInfo> members =
getMembers(resource.getGroupUUID(), new HashSet<AccountGroup.UUID>());
final List<AccountInfo> memberInfos = Lists.newArrayList(members.values());
Collections.sort(memberInfos, new Comparator<AccountInfo>() {
@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<Account.Id, AccountInfo> members =
getMembers(resource.getGroupUUID(), new HashSet<AccountGroup.UUID>());
final List<AccountInfo> memberInfos = Lists.newArrayList(members.values());
Collections.sort(memberInfos, new Comparator<AccountInfo>() {
@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<Account.Id, AccountInfo> getMembers(
final AccountGroup.UUID groupUUID,
final HashSet<AccountGroup.UUID> seenGroups) throws OrmException,
NoSuchGroupException {
final HashSet<AccountGroup.UUID> seenGroups) throws OrmException {
seenGroups.add(groupUUID);
final Map<Account.Id, AccountInfo> members = Maps.newHashMap();
@@ -96,8 +90,13 @@ public class ListMembers implements RestReadView<GroupResource> {
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) {