Use the term 'subgroup' instead of 'includes' where possible

The term 'includes' can be a bit confusing and misleading, in
particular as users which belong to a group aren't covered by it.
The term 'subgroup' more accurately describes the concept and
emphasizes the separation between groups and users.

All relevant parts of the server code as well as the documentation are
adapted to this new naming. Occurrences of the term in the REST or Java
API are left as is to not break compatibility.

Change-Id: Id952509eaf3e86e9361fb8f803cc160807b58239
This commit is contained in:
Alice Kober-Sotzek 2017-08-23 16:47:46 +02:00
parent b93e9b1ad9
commit 8a9d8a42eb
21 changed files with 205 additions and 211 deletions

View File

@ -116,7 +116,7 @@ client so they are generally disabled by default. Optional fields are:
[[includes]] [[includes]]
-- --
* `INCLUDES`: include list of directly included groups. * `INCLUDES`: include list of direct subgroups.
-- --
[[members]] [[members]]
@ -1129,13 +1129,13 @@ already a member of the group.
] ]
---- ----
[[delete-group-member]] [[remove-group-member]]
=== Delete Group Member === Remove Group Member
-- --
'DELETE /groups/link:#group-id[\{group-id\}]/members/link:rest-api-accounts.html#account-id[\{account-id\}]' 'DELETE /groups/link:#group-id[\{group-id\}]/members/link:rest-api-accounts.html#account-id[\{account-id\}]'
-- --
Deletes a user from a Gerrit internal group. Removes a user from a Gerrit internal group.
.Request .Request
---- ----
@ -1147,15 +1147,15 @@ Deletes a user from a Gerrit internal group.
HTTP/1.1 204 No Content HTTP/1.1 204 No Content
---- ----
[[delete-group-members]] [[remove-group-members]]
=== Delete Group Members === Remove Group Members
-- --
'POST /groups/link:#group-id[\{group-id\}]/members.delete' 'POST /groups/link:#group-id[\{group-id\}]/members.delete'
-- --
Delete one or several users from a Gerrit internal group. Removes one or several users from a Gerrit internal group.
The users to be deleted from the group must be provided in the request The users to be removed from the group must be provided in the request
body as a link:#members-input[MembersInput] entity. body as a link:#members-input[MembersInput] entity.
.Request .Request
@ -1176,16 +1176,16 @@ body as a link:#members-input[MembersInput] entity.
HTTP/1.1 204 No Content HTTP/1.1 204 No Content
---- ----
[[group-include-endpoints]] [[subgroup-endpoints]]
== Group Include Endpoints == Subgroup Endpoints
[[included-groups]] [[list-subgroups]]
=== List Included Groups === List Subgroups
-- --
'GET /groups/link:#group-id[\{group-id\}]/groups/' 'GET /groups/link:#group-id[\{group-id\}]/groups/'
-- --
Lists the directly included groups of a group. Lists the direct subgroups of a group.
As result a list of link:#group-info[GroupInfo] entries is returned. As result a list of link:#group-info[GroupInfo] entries is returned.
The entries in the list are sorted by group name and UUID. The entries in the list are sorted by group name and UUID.
@ -1217,13 +1217,13 @@ The entries in the list are sorted by group name and UUID.
] ]
---- ----
[[get-included-group]] [[get-subgroup]]
=== Get Included Group === Get Subgroup
-- --
'GET /groups/link:#group-id[\{group-id\}]/groups/link:#group-id[\{group-id\}]' 'GET /groups/link:#group-id[\{group-id\}]/groups/link:#group-id[\{group-id\}]'
-- --
Retrieves an included group. Retrieves a subgroup.
.Request .Request
---- ----
@ -1231,7 +1231,7 @@ Retrieves an included group.
---- ----
As response a link:#group-info[GroupInfo] entity is returned that As response a link:#group-info[GroupInfo] entity is returned that
describes the included group. describes the subgroup.
.Response .Response
---- ----
@ -1253,13 +1253,13 @@ describes the included group.
} }
---- ----
[[include-group]] [[add-subgroup]]
=== Include Group === Add Subgroup
-- --
'PUT /groups/link:#group-id[\{group-id\}]/groups/link:#group-id[\{group-id\}]' 'PUT /groups/link:#group-id[\{group-id\}]/groups/link:#group-id[\{group-id\}]'
-- --
Includes an internal or external group into a Gerrit internal group. Adds an internal or external group as subgroup to a Gerrit internal group.
External groups must be specified using the UUID. External groups must be specified using the UUID.
.Request .Request
@ -1268,7 +1268,7 @@ External groups must be specified using the UUID.
---- ----
As response a link:#group-info[GroupInfo] entity is returned that As response a link:#group-info[GroupInfo] entity is returned that
describes the included group. describes the subgroup.
.Response .Response
---- ----
@ -1290,11 +1290,11 @@ describes the included group.
} }
---- ----
The request also succeeds if the group is already included in this The request also succeeds if the group is already a subgroup of this
group, but then the HTTP response code is `200 OK`. group.
[[include-groups]] [[add-subgroups]]
=== Include Groups === Add Subgroups
-- --
'POST /groups/link:#group-id[\{group-id\}]/groups' 'POST /groups/link:#group-id[\{group-id\}]/groups'
-- --
@ -1305,10 +1305,10 @@ OR
'POST /groups/link:#group-id[\{group-id\}]/groups.add' 'POST /groups/link:#group-id[\{group-id\}]/groups.add'
-- --
Includes one or several groups into a Gerrit internal group. Adds one or several groups as subgroups to a Gerrit internal group.
The groups to be included into the group must be provided in the The subgroups to be added must be provided in the request body as a
request body as a link:#groups-input[GroupsInput] entity. link:#groups-input[GroupsInput] entity.
.Request .Request
---- ----
@ -1327,8 +1327,8 @@ As response a list of link:#group-info[GroupInfo] entities is
returned that describes the groups that were specified in the returned that describes the groups that were specified in the
link:#groups-input[GroupsInput]. A link:#group-info[GroupInfo] entity link:#groups-input[GroupsInput]. A link:#group-info[GroupInfo] entity
is returned for each group specified in the input, independently of is returned for each group specified in the input, independently of
whether the group was newly included into the group or whether the whether the group was newly added as subgroup or whether the
group was already included in the group. group was already a subgroup of the group.
.Response .Response
---- ----
@ -1363,13 +1363,13 @@ group was already included in the group.
] ]
---- ----
[[delete-included-group]] [[remove-subgroup]]
=== Delete Included Group === Remove Subgroup
-- --
'DELETE /groups/link:#group-id[\{group-id\}]/groups/link:#group-id[\{group-id\}]' 'DELETE /groups/link:#group-id[\{group-id\}]/groups/link:#group-id[\{group-id\}]'
-- --
Deletes an included group from a Gerrit internal group. Removes a subgroup from a Gerrit internal group.
.Request .Request
---- ----
@ -1381,16 +1381,16 @@ Deletes an included group from a Gerrit internal group.
HTTP/1.1 204 No Content HTTP/1.1 204 No Content
---- ----
[[delete-included-groups]] [[remove-subgroups]]
=== Delete Included Groups === Remove Subgroups
-- --
'POST /groups/link:#group-id[\{group-id\}]/groups.delete' 'POST /groups/link:#group-id[\{group-id\}]/groups.delete'
-- --
Delete one or several included groups from a Gerrit internal group. Removes one or several subgroups from a Gerrit internal group.
The groups to be deleted from the group must be provided in the request The subgroups to be removed must be provided in the request body as a
body as a link:#groups-input[GroupsInput] entity. link:#groups-input[GroupsInput] entity.
.Request .Request
---- ----
@ -1495,9 +1495,9 @@ A list of link:rest-api-accounts.html#account-info[AccountInfo]
entities describing the direct members. + entities describing the direct members. +
Only set if link:#members[members] are requested. Only set if link:#members[members] are requested.
|`includes` |optional, only for internal groups| |`includes` |optional, only for internal groups|
A list of link:#group-info[GroupInfo] entities describing the directly A list of link:#group-info[GroupInfo] entities describing the direct
included groups. + subgroups. +
Only set if link:#includes[included groups] are requested. Only set if link:#includes[subgroups] are requested.
|=========================== |===========================
The type of a group can be deduced from the group's UUID: The type of a group can be deduced from the group's UUID:

View File

@ -109,15 +109,15 @@ public interface GroupApi {
void removeMembers(String... members) throws RestApiException; void removeMembers(String... members) throws RestApiException;
/** /**
* List included groups. * Lists the subgroups of this group.
* *
* @return included groups. * @return the found subgroups
* @throws RestApiException * @throws RestApiException
*/ */
List<GroupInfo> includedGroups() throws RestApiException; List<GroupInfo> includedGroups() throws RestApiException;
/** /**
* Add groups to be included in this one. * Adds subgroups to this group.
* *
* @param groups list of group identifiers, in any format accepted by {@link Groups#id(String)} * @param groups list of group identifiers, in any format accepted by {@link Groups#id(String)}
* @throws RestApiException * @throws RestApiException
@ -125,7 +125,7 @@ public interface GroupApi {
void addGroups(String... groups) throws RestApiException; void addGroups(String... groups) throws RestApiException;
/** /**
* Remove included groups from this one. * Removes subgroups from this group.
* *
* @param groups list of group identifiers, in any format accepted by {@link Groups#id(String)} * @param groups list of group identifiers, in any format accepted by {@link Groups#id(String)}
* @throws RestApiException * @throws RestApiException

View File

@ -239,9 +239,9 @@ public class GroupCacheImpl implements GroupCache {
ImmutableSet<Account.Id> members = ImmutableSet<Account.Id> members =
groups.getMembers(db, groupUuid).collect(toImmutableSet()); groups.getMembers(db, groupUuid).collect(toImmutableSet());
ImmutableSet<AccountGroup.UUID> includes = ImmutableSet<AccountGroup.UUID> subgroups =
groups.getIncludes(db, groupUuid).collect(toImmutableSet()); groups.getSubgroups(db, groupUuid).collect(toImmutableSet());
return accountGroup.map(group -> InternalGroup.create(group, members, includes)); return accountGroup.map(group -> InternalGroup.create(group, members, subgroups));
} }
} }
} }

View File

@ -150,7 +150,7 @@ public class GroupIncludeCacheImpl implements GroupIncludeCache {
public ImmutableList<AccountGroup.UUID> load(AccountGroup.UUID key) public ImmutableList<AccountGroup.UUID> load(AccountGroup.UUID key)
throws OrmException, NoSuchGroupException { throws OrmException, NoSuchGroupException {
try (ReviewDb db = schema.open()) { try (ReviewDb db = schema.open()) {
return groups.getIncludes(db, key).collect(toImmutableList()); return groups.getSubgroups(db, key).collect(toImmutableList());
} }
} }
} }

View File

@ -117,7 +117,7 @@ public class GroupMembers {
Set<Account> indirectMembers = new HashSet<>(); Set<Account> indirectMembers = new HashSet<>();
if (groupControl.canSeeGroup()) { if (groupControl.canSeeGroup()) {
for (AccountGroup.UUID subgroupUuid : group.getIncludes()) { for (AccountGroup.UUID subgroupUuid : group.getSubgroups()) {
if (!seen.contains(subgroupUuid)) { if (!seen.contains(subgroupUuid)) {
indirectMembers.addAll(listAccounts(subgroupUuid, project, seen)); indirectMembers.addAll(listAccounts(subgroupUuid, project, seen));
} }

View File

@ -97,7 +97,7 @@ public class IncludingGroupMembership implements GroupMembership {
if (!group.isPresent()) { if (!group.isPresent()) {
continue; continue;
} }
if (search(group.get().getIncludes())) { if (search(group.get().getSubgroups())) {
memberOf.put(id, true); memberOf.put(id, true);
return true; return true;
} }

View File

@ -22,10 +22,10 @@ import com.google.gerrit.extensions.common.GroupAuditEventInfo;
import com.google.gerrit.extensions.common.GroupInfo; import com.google.gerrit.extensions.common.GroupInfo;
import com.google.gerrit.extensions.common.GroupOptionsInfo; import com.google.gerrit.extensions.common.GroupOptionsInfo;
import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.server.group.AddIncludedGroups;
import com.google.gerrit.server.group.AddMembers; import com.google.gerrit.server.group.AddMembers;
import com.google.gerrit.server.group.DeleteIncludedGroups; import com.google.gerrit.server.group.AddSubgroups;
import com.google.gerrit.server.group.DeleteMembers; import com.google.gerrit.server.group.DeleteMembers;
import com.google.gerrit.server.group.DeleteSubgroups;
import com.google.gerrit.server.group.GetAuditLog; import com.google.gerrit.server.group.GetAuditLog;
import com.google.gerrit.server.group.GetDescription; import com.google.gerrit.server.group.GetDescription;
import com.google.gerrit.server.group.GetDetail; import com.google.gerrit.server.group.GetDetail;
@ -35,8 +35,8 @@ import com.google.gerrit.server.group.GetOptions;
import com.google.gerrit.server.group.GetOwner; import com.google.gerrit.server.group.GetOwner;
import com.google.gerrit.server.group.GroupResource; import com.google.gerrit.server.group.GroupResource;
import com.google.gerrit.server.group.Index; import com.google.gerrit.server.group.Index;
import com.google.gerrit.server.group.ListIncludedGroups;
import com.google.gerrit.server.group.ListMembers; import com.google.gerrit.server.group.ListMembers;
import com.google.gerrit.server.group.ListSubgroups;
import com.google.gerrit.server.group.PutDescription; import com.google.gerrit.server.group.PutDescription;
import com.google.gerrit.server.group.PutName; import com.google.gerrit.server.group.PutName;
import com.google.gerrit.server.group.PutOptions; import com.google.gerrit.server.group.PutOptions;
@ -64,9 +64,9 @@ class GroupApiImpl implements GroupApi {
private final ListMembers listMembers; private final ListMembers listMembers;
private final AddMembers addMembers; private final AddMembers addMembers;
private final DeleteMembers deleteMembers; private final DeleteMembers deleteMembers;
private final ListIncludedGroups listGroups; private final ListSubgroups listSubgroups;
private final AddIncludedGroups addGroups; private final AddSubgroups addSubgroups;
private final DeleteIncludedGroups deleteGroups; private final DeleteSubgroups deleteSubgroups;
private final GetAuditLog getAuditLog; private final GetAuditLog getAuditLog;
private final GroupResource rsrc; private final GroupResource rsrc;
private final Index index; private final Index index;
@ -86,9 +86,9 @@ class GroupApiImpl implements GroupApi {
ListMembers listMembers, ListMembers listMembers,
AddMembers addMembers, AddMembers addMembers,
DeleteMembers deleteMembers, DeleteMembers deleteMembers,
ListIncludedGroups listGroups, ListSubgroups listSubgroups,
AddIncludedGroups addGroups, AddSubgroups addSubgroups,
DeleteIncludedGroups deleteGroups, DeleteSubgroups deleteSubgroups,
GetAuditLog getAuditLog, GetAuditLog getAuditLog,
Index index, Index index,
@Assisted GroupResource rsrc) { @Assisted GroupResource rsrc) {
@ -105,9 +105,9 @@ class GroupApiImpl implements GroupApi {
this.listMembers = listMembers; this.listMembers = listMembers;
this.addMembers = addMembers; this.addMembers = addMembers;
this.deleteMembers = deleteMembers; this.deleteMembers = deleteMembers;
this.listGroups = listGroups; this.listSubgroups = listSubgroups;
this.addGroups = addGroups; this.addSubgroups = addSubgroups;
this.deleteGroups = deleteGroups; this.deleteSubgroups = deleteSubgroups;
this.getAuditLog = getAuditLog; this.getAuditLog = getAuditLog;
this.index = index; this.index = index;
this.rsrc = rsrc; this.rsrc = rsrc;
@ -233,27 +233,27 @@ class GroupApiImpl implements GroupApi {
@Override @Override
public List<GroupInfo> includedGroups() throws RestApiException { public List<GroupInfo> includedGroups() throws RestApiException {
try { try {
return listGroups.apply(rsrc); return listSubgroups.apply(rsrc);
} catch (Exception e) { } catch (Exception e) {
throw asRestApiException("Cannot list included groups", e); throw asRestApiException("Cannot list subgroups", e);
} }
} }
@Override @Override
public void addGroups(String... groups) throws RestApiException { public void addGroups(String... groups) throws RestApiException {
try { try {
addGroups.apply(rsrc, AddIncludedGroups.Input.fromGroups(Arrays.asList(groups))); addSubgroups.apply(rsrc, AddSubgroups.Input.fromGroups(Arrays.asList(groups)));
} catch (Exception e) { } catch (Exception e) {
throw asRestApiException("Cannot add group members", e); throw asRestApiException("Cannot add subgroups", e);
} }
} }
@Override @Override
public void removeGroups(String... groups) throws RestApiException { public void removeGroups(String... groups) throws RestApiException {
try { try {
deleteGroups.apply(rsrc, AddIncludedGroups.Input.fromGroups(Arrays.asList(groups))); deleteSubgroups.apply(rsrc, AddSubgroups.Input.fromGroups(Arrays.asList(groups)));
} catch (Exception e) { } catch (Exception e) {
throw asRestApiException("Cannot remove group members", e); throw asRestApiException("Cannot remove subgroups", e);
} }
} }

View File

@ -29,7 +29,7 @@ import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.account.GroupControl; import com.google.gerrit.server.account.GroupControl;
import com.google.gerrit.server.group.AddIncludedGroups.Input; import com.google.gerrit.server.group.AddSubgroups.Input;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
@ -41,7 +41,7 @@ import java.util.List;
import java.util.Set; import java.util.Set;
@Singleton @Singleton
public class AddIncludedGroups implements RestModifyView<GroupResource, Input> { public class AddSubgroups implements RestModifyView<GroupResource, Input> {
public static class Input { public static class Input {
@DefaultInput String _oneGroup; @DefaultInput String _oneGroup;
@ -73,7 +73,7 @@ public class AddIncludedGroups implements RestModifyView<GroupResource, Input> {
private final GroupJson json; private final GroupJson json;
@Inject @Inject
public AddIncludedGroups( public AddSubgroups(
GroupsCollection groupsCollection, GroupsCollection groupsCollection,
Provider<ReviewDb> db, Provider<ReviewDb> db,
@UserInitiated Provider<GroupsUpdate> groupsUpdateProvider, @UserInitiated Provider<GroupsUpdate> groupsUpdateProvider,
@ -98,30 +98,30 @@ public class AddIncludedGroups implements RestModifyView<GroupResource, Input> {
} }
List<GroupInfo> result = new ArrayList<>(); List<GroupInfo> result = new ArrayList<>();
Set<AccountGroup.UUID> includedGroupUuids = new HashSet<>(); Set<AccountGroup.UUID> subgroupUuids = new HashSet<>();
for (String includedGroup : input.groups) { for (String subgroupIdentifier : input.groups) {
GroupDescription.Basic d = groupsCollection.parse(includedGroup); GroupDescription.Basic subgroup = groupsCollection.parse(subgroupIdentifier);
includedGroupUuids.add(d.getGroupUUID()); subgroupUuids.add(subgroup.getGroupUUID());
result.add(json.format(d)); result.add(json.format(subgroup));
} }
AccountGroup.UUID groupUuid = group.getGroupUUID(); AccountGroup.UUID groupUuid = group.getGroupUUID();
try { try {
groupsUpdateProvider.get().addIncludedGroups(db.get(), groupUuid, includedGroupUuids); groupsUpdateProvider.get().addSubgroups(db.get(), groupUuid, subgroupUuids);
} catch (NoSuchGroupException e) { } catch (NoSuchGroupException e) {
throw new ResourceNotFoundException(String.format("Group %s not found", groupUuid)); throw new ResourceNotFoundException(String.format("Group %s not found", groupUuid));
} }
return result; return result;
} }
static class PutIncludedGroup implements RestModifyView<GroupResource, PutIncludedGroup.Input> { static class PutSubgroup implements RestModifyView<GroupResource, PutSubgroup.Input> {
static class Input {} static class Input {}
private final AddIncludedGroups put; private final AddSubgroups addSubgroups;
private final String id; private final String id;
PutIncludedGroup(AddIncludedGroups put, String id) { PutSubgroup(AddSubgroups addSubgroups, String id) {
this.put = put; this.addSubgroups = addSubgroups;
this.id = id; this.id = id;
} }
@ -129,10 +129,10 @@ public class AddIncludedGroups implements RestModifyView<GroupResource, Input> {
public GroupInfo apply(GroupResource resource, Input input) public GroupInfo apply(GroupResource resource, Input input)
throws AuthException, MethodNotAllowedException, ResourceNotFoundException, OrmException, throws AuthException, MethodNotAllowedException, ResourceNotFoundException, OrmException,
IOException { IOException {
AddIncludedGroups.Input in = new AddIncludedGroups.Input(); AddSubgroups.Input in = new AddSubgroups.Input();
in.groups = ImmutableList.of(id); in.groups = ImmutableList.of(id);
try { try {
List<GroupInfo> list = put.apply(resource, in); List<GroupInfo> list = addSubgroups.apply(resource, in);
if (list.size() == 1) { if (list.size() == 1) {
return list.get(0); return list.get(0);
} }
@ -144,18 +144,16 @@ public class AddIncludedGroups implements RestModifyView<GroupResource, Input> {
} }
@Singleton @Singleton
static class UpdateIncludedGroup static class UpdateSubgroup implements RestModifyView<SubgroupResource, PutSubgroup.Input> {
implements RestModifyView<IncludedGroupResource, PutIncludedGroup.Input> { private final Provider<GetSubgroup> get;
private final Provider<GetIncludedGroup> get;
@Inject @Inject
UpdateIncludedGroup(Provider<GetIncludedGroup> get) { UpdateSubgroup(Provider<GetSubgroup> get) {
this.get = get; this.get = get;
} }
@Override @Override
public GroupInfo apply(IncludedGroupResource resource, PutIncludedGroup.Input input) public GroupInfo apply(SubgroupResource resource, PutSubgroup.Input input) throws OrmException {
throws OrmException {
// Do nothing, the group is already included. // Do nothing, the group is already included.
return get.get().apply(resource); return get.get().apply(resource);
} }

View File

@ -26,7 +26,7 @@ import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.account.GroupControl; import com.google.gerrit.server.account.GroupControl;
import com.google.gerrit.server.group.AddIncludedGroups.Input; import com.google.gerrit.server.group.AddSubgroups.Input;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
@ -36,13 +36,13 @@ import java.util.HashSet;
import java.util.Set; import java.util.Set;
@Singleton @Singleton
public class DeleteIncludedGroups implements RestModifyView<GroupResource, Input> { public class DeleteSubgroups implements RestModifyView<GroupResource, Input> {
private final GroupsCollection groupsCollection; private final GroupsCollection groupsCollection;
private final Provider<ReviewDb> db; private final Provider<ReviewDb> db;
private final Provider<GroupsUpdate> groupsUpdateProvider; private final Provider<GroupsUpdate> groupsUpdateProvider;
@Inject @Inject
DeleteIncludedGroups( DeleteSubgroups(
GroupsCollection groupsCollection, GroupsCollection groupsCollection,
Provider<ReviewDb> db, Provider<ReviewDb> db,
@UserInitiated Provider<GroupsUpdate> groupsUpdateProvider) { @UserInitiated Provider<GroupsUpdate> groupsUpdateProvider) {
@ -65,15 +65,15 @@ public class DeleteIncludedGroups implements RestModifyView<GroupResource, Input
String.format("Cannot delete groups from group %s", internalGroup.getName())); String.format("Cannot delete groups from group %s", internalGroup.getName()));
} }
Set<AccountGroup.UUID> internalGroupsToRemove = new HashSet<>(); Set<AccountGroup.UUID> subgroupsToRemove = new HashSet<>();
for (String includedGroup : input.groups) { for (String subgroupIdentifier : input.groups) {
GroupDescription.Basic d = groupsCollection.parse(includedGroup); GroupDescription.Basic subgroup = groupsCollection.parse(subgroupIdentifier);
internalGroupsToRemove.add(d.getGroupUUID()); subgroupsToRemove.add(subgroup.getGroupUUID());
} }
AccountGroup.UUID groupUuid = internalGroup.getGroupUUID(); AccountGroup.UUID groupUuid = internalGroup.getGroupUUID();
try { try {
groupsUpdateProvider.get().deleteIncludedGroups(db.get(), groupUuid, internalGroupsToRemove); groupsUpdateProvider.get().removeSubgroups(db.get(), groupUuid, subgroupsToRemove);
} catch (NoSuchGroupException e) { } catch (NoSuchGroupException e) {
throw new ResourceNotFoundException(String.format("Group %s not found", groupUuid)); throw new ResourceNotFoundException(String.format("Group %s not found", groupUuid));
} }
@ -82,22 +82,21 @@ public class DeleteIncludedGroups implements RestModifyView<GroupResource, Input
} }
@Singleton @Singleton
static class DeleteIncludedGroup static class DeleteSubgroup implements RestModifyView<SubgroupResource, DeleteSubgroup.Input> {
implements RestModifyView<IncludedGroupResource, DeleteIncludedGroup.Input> {
static class Input {} static class Input {}
private final Provider<DeleteIncludedGroups> delete; private final Provider<DeleteSubgroups> delete;
@Inject @Inject
DeleteIncludedGroup(Provider<DeleteIncludedGroups> delete) { DeleteSubgroup(Provider<DeleteSubgroups> delete) {
this.delete = delete; this.delete = delete;
} }
@Override @Override
public Response<?> apply(IncludedGroupResource resource, Input input) public Response<?> apply(SubgroupResource resource, Input input)
throws AuthException, MethodNotAllowedException, UnprocessableEntityException, OrmException, throws AuthException, MethodNotAllowedException, UnprocessableEntityException, OrmException,
ResourceNotFoundException, IOException { ResourceNotFoundException, IOException {
AddIncludedGroups.Input in = new AddIncludedGroups.Input(); AddSubgroups.Input in = new AddSubgroups.Input();
in.groups = ImmutableList.of(resource.getMember().get()); in.groups = ImmutableList.of(resource.getMember().get());
return delete.get().apply(resource, in); return delete.get().apply(resource, in);
} }

View File

@ -21,16 +21,16 @@ import com.google.inject.Inject;
import com.google.inject.Singleton; import com.google.inject.Singleton;
@Singleton @Singleton
public class GetIncludedGroup implements RestReadView<IncludedGroupResource> { public class GetSubgroup implements RestReadView<SubgroupResource> {
private final GroupJson json; private final GroupJson json;
@Inject @Inject
GetIncludedGroup(GroupJson json) { GetSubgroup(GroupJson json) {
this.json = json; this.json = json;
} }
@Override @Override
public GroupInfo apply(IncludedGroupResource rsrc) throws OrmException { public GroupInfo apply(SubgroupResource rsrc) throws OrmException {
return json.format(rsrc.getMemberDescription()); return json.format(rsrc.getMemberDescription());
} }
} }

View File

@ -45,7 +45,7 @@ public class GroupJson {
private final GroupBackend groupBackend; private final GroupBackend groupBackend;
private final GroupControl.Factory groupControlFactory; private final GroupControl.Factory groupControlFactory;
private final Provider<ListMembers> listMembers; private final Provider<ListMembers> listMembers;
private final Provider<ListIncludedGroups> listIncludes; private final Provider<ListSubgroups> listSubgroups;
private EnumSet<ListGroupsOption> options; private EnumSet<ListGroupsOption> options;
@Inject @Inject
@ -53,11 +53,11 @@ public class GroupJson {
GroupBackend groupBackend, GroupBackend groupBackend,
GroupControl.Factory groupControlFactory, GroupControl.Factory groupControlFactory,
Provider<ListMembers> listMembers, Provider<ListMembers> listMembers,
Provider<ListIncludedGroups> listIncludes) { Provider<ListSubgroups> listSubgroups) {
this.groupBackend = groupBackend; this.groupBackend = groupBackend;
this.groupControlFactory = groupControlFactory; this.groupControlFactory = groupControlFactory;
this.listMembers = listMembers; this.listMembers = listMembers;
this.listIncludes = listIncludes; this.listSubgroups = listSubgroups;
options = EnumSet.noneOf(ListGroupsOption.class); options = EnumSet.noneOf(ListGroupsOption.class);
} }
@ -74,7 +74,7 @@ public class GroupJson {
public GroupInfo format(GroupResource rsrc) throws OrmException { public GroupInfo format(GroupResource rsrc) throws OrmException {
GroupInfo info = init(rsrc.getGroup()); GroupInfo info = init(rsrc.getGroup());
initMembersAndIncludes(rsrc, info); initMembersAndSubgroups(rsrc, info);
return info; return info;
} }
@ -82,7 +82,7 @@ public class GroupJson {
GroupInfo info = init(group); GroupInfo info = init(group);
if (options.contains(MEMBERS) || options.contains(INCLUDES)) { if (options.contains(MEMBERS) || options.contains(INCLUDES)) {
GroupResource rsrc = new GroupResource(groupControlFactory.controlFor(group)); GroupResource rsrc = new GroupResource(groupControlFactory.controlFor(group));
initMembersAndIncludes(rsrc, info); initMembersAndSubgroups(rsrc, info);
} }
return info; return info;
} }
@ -116,7 +116,8 @@ public class GroupJson {
return group instanceof GroupDescription.Internal; return group instanceof GroupDescription.Internal;
} }
private GroupInfo initMembersAndIncludes(GroupResource rsrc, GroupInfo info) throws OrmException { private GroupInfo initMembersAndSubgroups(GroupResource rsrc, GroupInfo info)
throws OrmException {
if (!rsrc.isInternalGroup()) { if (!rsrc.isInternalGroup()) {
return info; return info;
} }
@ -126,7 +127,7 @@ public class GroupJson {
} }
if (options.contains(INCLUDES)) { if (options.contains(INCLUDES)) {
info.includes = listIncludes.get().apply(rsrc); info.includes = listSubgroups.get().apply(rsrc);
} }
return info; return info;
} catch (MethodNotAllowedException e) { } catch (MethodNotAllowedException e) {

View File

@ -145,16 +145,16 @@ public class Groups {
* *
* @param db the {@code ReviewDb} instance to use for lookups * @param db the {@code ReviewDb} instance to use for lookups
* @param parentGroupUuid the UUID of the parent group * @param parentGroupUuid the UUID of the parent group
* @param includedGroupUuid the UUID of the subgroup * @param subgroupUuid the UUID of the subgroup
* @return {@code true} if the group is a subgroup of the other group, or else {@code false} * @return {@code true} if the group is a subgroup of the other group, or else {@code false}
* @throws OrmException if an error occurs while reading from ReviewDb * @throws OrmException if an error occurs while reading from ReviewDb
* @throws NoSuchGroupException if the specified parent group doesn't exist * @throws NoSuchGroupException if the specified parent group doesn't exist
*/ */
public boolean isIncluded( public boolean isSubgroup(
ReviewDb db, AccountGroup.UUID parentGroupUuid, AccountGroup.UUID includedGroupUuid) ReviewDb db, AccountGroup.UUID parentGroupUuid, AccountGroup.UUID subgroupUuid)
throws OrmException, NoSuchGroupException { throws OrmException, NoSuchGroupException {
AccountGroup parentGroup = getExistingGroup(db, parentGroupUuid); AccountGroup parentGroup = getExistingGroup(db, parentGroupUuid);
AccountGroupById.Key key = new AccountGroupById.Key(parentGroup.getId(), includedGroupUuid); AccountGroupById.Key key = new AccountGroupById.Key(parentGroup.getId(), subgroupUuid);
return db.accountGroupById().get(key) != null; return db.accountGroupById().get(key) != null;
} }
@ -191,7 +191,7 @@ public class Groups {
* @throws OrmException if an error occurs while reading from ReviewDb * @throws OrmException if an error occurs while reading from ReviewDb
* @throws NoSuchGroupException if the specified parent group doesn't exist * @throws NoSuchGroupException if the specified parent group doesn't exist
*/ */
public Stream<AccountGroup.UUID> getIncludes(ReviewDb db, AccountGroup.UUID groupUuid) public Stream<AccountGroup.UUID> getSubgroups(ReviewDb db, AccountGroup.UUID groupUuid)
throws OrmException, NoSuchGroupException { throws OrmException, NoSuchGroupException {
AccountGroup group = getExistingGroup(db, groupUuid); AccountGroup group = getExistingGroup(db, groupUuid);
ResultSet<AccountGroupById> accountGroupByIds = db.accountGroupById().byGroup(group.getId()); ResultSet<AccountGroupById> accountGroupByIds = db.accountGroupById().byGroup(group.getId());
@ -226,14 +226,14 @@ public class Groups {
* exist. This method doesn't check whether the parent groups exist. * exist. This method doesn't check whether the parent groups exist.
* *
* @param db the {@code ReviewDb} instance to use for lookups * @param db the {@code ReviewDb} instance to use for lookups
* @param includedGroupUuid the UUID of the subgroup * @param subgroupUuid the UUID of the subgroup
* @return a stream of the IDs of the parent groups * @return a stream of the IDs of the parent groups
* @throws OrmException if an error occurs while reading from ReviewDb * @throws OrmException if an error occurs while reading from ReviewDb
*/ */
public Stream<AccountGroup.Id> getParentGroups(ReviewDb db, AccountGroup.UUID includedGroupUuid) public Stream<AccountGroup.Id> getParentGroups(ReviewDb db, AccountGroup.UUID subgroupUuid)
throws OrmException { throws OrmException {
ResultSet<AccountGroupById> accountGroupByIds = ResultSet<AccountGroupById> accountGroupByIds =
db.accountGroupById().byIncludeUUID(includedGroupUuid); db.accountGroupById().byIncludeUUID(subgroupUuid);
return Streams.stream(accountGroupByIds).map(AccountGroupById::getGroupId); return Streams.stream(accountGroupByIds).map(AccountGroupById::getGroupId);
} }

View File

@ -349,35 +349,35 @@ public class GroupsUpdate {
* *
* @param db the {@code ReviewDb} instance to update * @param db the {@code ReviewDb} instance to update
* @param parentGroupUuid the UUID of the parent group * @param parentGroupUuid the UUID of the parent group
* @param includedGroupUuids a set of IDs of the groups to add as subgroups * @param subgroupUuids a set of IDs of the groups to add as subgroups
* @throws OrmException if an error occurs while reading/writing from/to ReviewDb * @throws OrmException if an error occurs while reading/writing from/to ReviewDb
* @throws IOException if the parent group couldn't be indexed * @throws IOException if the parent group couldn't be indexed
* @throws NoSuchGroupException if the specified parent group doesn't exist * @throws NoSuchGroupException if the specified parent group doesn't exist
*/ */
public void addIncludedGroups( public void addSubgroups(
ReviewDb db, AccountGroup.UUID parentGroupUuid, Set<AccountGroup.UUID> includedGroupUuids) ReviewDb db, AccountGroup.UUID parentGroupUuid, Set<AccountGroup.UUID> subgroupUuids)
throws OrmException, NoSuchGroupException, IOException { throws OrmException, NoSuchGroupException, IOException {
AccountGroup parentGroup = groups.getExistingGroup(db, parentGroupUuid); AccountGroup parentGroup = groups.getExistingGroup(db, parentGroupUuid);
AccountGroup.Id parentGroupId = parentGroup.getId(); AccountGroup.Id parentGroupId = parentGroup.getId();
Set<AccountGroupById> newIncludedGroups = new HashSet<>(); Set<AccountGroupById> newSubgroups = new HashSet<>();
for (AccountGroup.UUID includedGroupUuid : includedGroupUuids) { for (AccountGroup.UUID includedGroupUuid : subgroupUuids) {
boolean isIncluded = groups.isIncluded(db, parentGroupUuid, includedGroupUuid); boolean isSubgroup = groups.isSubgroup(db, parentGroupUuid, includedGroupUuid);
if (!isIncluded) { if (!isSubgroup) {
AccountGroupById.Key key = new AccountGroupById.Key(parentGroupId, includedGroupUuid); AccountGroupById.Key key = new AccountGroupById.Key(parentGroupId, includedGroupUuid);
newIncludedGroups.add(new AccountGroupById(key)); newSubgroups.add(new AccountGroupById(key));
} }
} }
if (newIncludedGroups.isEmpty()) { if (newSubgroups.isEmpty()) {
return; return;
} }
if (currentUser != null) { if (currentUser != null) {
auditService.dispatchAddGroupsToGroup(currentUser.getAccountId(), newIncludedGroups); auditService.dispatchAddGroupsToGroup(currentUser.getAccountId(), newSubgroups);
} }
db.accountGroupById().insert(newIncludedGroups); db.accountGroupById().insert(newSubgroups);
groupCache.evict(parentGroup.getGroupUUID(), parentGroup.getId(), parentGroup.getNameKey()); groupCache.evict(parentGroup.getGroupUUID(), parentGroup.getId(), parentGroup.getNameKey());
for (AccountGroupById newIncludedGroup : newIncludedGroups) { for (AccountGroupById newIncludedGroup : newSubgroups) {
groupIncludeCache.evictParentGroupsOf(newIncludedGroup.getIncludeUUID()); groupIncludeCache.evictParentGroupsOf(newIncludedGroup.getIncludeUUID());
} }
groupIncludeCache.evictSubgroupsOf(parentGroupUuid); groupIncludeCache.evictSubgroupsOf(parentGroupUuid);
@ -392,36 +392,35 @@ public class GroupsUpdate {
* *
* @param db the {@code ReviewDb} instance to update * @param db the {@code ReviewDb} instance to update
* @param parentGroupUuid the UUID of the parent group * @param parentGroupUuid the UUID of the parent group
* @param includedGroupUuids a set of IDs of the subgroups to remove from the parent group * @param subgroupUuids a set of IDs of the subgroups to remove from the parent group
* @throws OrmException if an error occurs while reading/writing from/to ReviewDb * @throws OrmException if an error occurs while reading/writing from/to ReviewDb
* @throws IOException if the parent group couldn't be indexed * @throws IOException if the parent group couldn't be indexed
* @throws NoSuchGroupException if the specified parent group doesn't exist * @throws NoSuchGroupException if the specified parent group doesn't exist
*/ */
public void deleteIncludedGroups( public void removeSubgroups(
ReviewDb db, AccountGroup.UUID parentGroupUuid, Set<AccountGroup.UUID> includedGroupUuids) ReviewDb db, AccountGroup.UUID parentGroupUuid, Set<AccountGroup.UUID> subgroupUuids)
throws OrmException, NoSuchGroupException, IOException { throws OrmException, NoSuchGroupException, IOException {
AccountGroup parentGroup = groups.getExistingGroup(db, parentGroupUuid); AccountGroup parentGroup = groups.getExistingGroup(db, parentGroupUuid);
AccountGroup.Id parentGroupId = parentGroup.getId(); AccountGroup.Id parentGroupId = parentGroup.getId();
Set<AccountGroupById> includedGroupsToRemove = new HashSet<>(); Set<AccountGroupById> subgroupsToRemove = new HashSet<>();
for (AccountGroup.UUID includedGroupUuid : includedGroupUuids) { for (AccountGroup.UUID subgroupUuid : subgroupUuids) {
boolean isIncluded = groups.isIncluded(db, parentGroupUuid, includedGroupUuid); boolean isSubgroup = groups.isSubgroup(db, parentGroupUuid, subgroupUuid);
if (isIncluded) { if (isSubgroup) {
AccountGroupById.Key key = new AccountGroupById.Key(parentGroupId, includedGroupUuid); AccountGroupById.Key key = new AccountGroupById.Key(parentGroupId, subgroupUuid);
includedGroupsToRemove.add(new AccountGroupById(key)); subgroupsToRemove.add(new AccountGroupById(key));
} }
} }
if (includedGroupsToRemove.isEmpty()) { if (subgroupsToRemove.isEmpty()) {
return; return;
} }
if (currentUser != null) { if (currentUser != null) {
auditService.dispatchDeleteGroupsFromGroup( auditService.dispatchDeleteGroupsFromGroup(currentUser.getAccountId(), subgroupsToRemove);
currentUser.getAccountId(), includedGroupsToRemove);
} }
db.accountGroupById().delete(includedGroupsToRemove); db.accountGroupById().delete(subgroupsToRemove);
groupCache.evict(parentGroup.getGroupUUID(), parentGroup.getId(), parentGroup.getNameKey()); groupCache.evict(parentGroup.getGroupUUID(), parentGroup.getId(), parentGroup.getNameKey());
for (AccountGroupById groupToRemove : includedGroupsToRemove) { for (AccountGroupById groupToRemove : subgroupsToRemove) {
groupIncludeCache.evictParentGroupsOf(groupToRemove.getIncludeUUID()); groupIncludeCache.evictParentGroupsOf(groupToRemove.getIncludeUUID());
} }
groupIncludeCache.evictSubgroupsOf(parentGroupUuid); groupIncludeCache.evictSubgroupsOf(parentGroupUuid);

View File

@ -27,7 +27,7 @@ public abstract class InternalGroup {
public static InternalGroup create( public static InternalGroup create(
AccountGroup accountGroup, AccountGroup accountGroup,
ImmutableSet<Account.Id> members, ImmutableSet<Account.Id> members,
ImmutableSet<AccountGroup.UUID> includes) { ImmutableSet<AccountGroup.UUID> subgroups) {
return new AutoValue_InternalGroup( return new AutoValue_InternalGroup(
accountGroup.getId(), accountGroup.getId(),
accountGroup.getNameKey(), accountGroup.getNameKey(),
@ -37,7 +37,7 @@ public abstract class InternalGroup {
accountGroup.getGroupUUID(), accountGroup.getGroupUUID(),
accountGroup.getCreatedOn(), accountGroup.getCreatedOn(),
members, members,
includes); subgroups);
} }
public abstract AccountGroup.Id getId(); public abstract AccountGroup.Id getId();
@ -61,5 +61,5 @@ public abstract class InternalGroup {
public abstract ImmutableSet<Account.Id> getMembers(); public abstract ImmutableSet<Account.Id> getMembers();
public abstract ImmutableSet<AccountGroup.UUID> getIncludes(); public abstract ImmutableSet<AccountGroup.UUID> getSubgroups();
} }

View File

@ -96,7 +96,7 @@ public class ListMembers implements RestReadView<GroupResource> {
Set<Account.Id> indirectMembers = new HashSet<>(); Set<Account.Id> indirectMembers = new HashSet<>();
if (recursive && groupControl.canSeeGroup()) { if (recursive && groupControl.canSeeGroup()) {
for (AccountGroup.UUID subgroupUuid : group.getIncludes()) { for (AccountGroup.UUID subgroupUuid : group.getSubgroups()) {
if (!seenGroups.contains(subgroupUuid)) { if (!seenGroups.contains(subgroupUuid)) {
indirectMembers.addAll(getMembers(subgroupUuid, seenGroups)); indirectMembers.addAll(getMembers(subgroupUuid, seenGroups));
} }

View File

@ -35,15 +35,15 @@ import java.util.List;
import org.slf4j.Logger; import org.slf4j.Logger;
@Singleton @Singleton
public class ListIncludedGroups implements RestReadView<GroupResource> { public class ListSubgroups implements RestReadView<GroupResource> {
private static final Logger log = org.slf4j.LoggerFactory.getLogger(ListIncludedGroups.class); private static final Logger log = org.slf4j.LoggerFactory.getLogger(ListSubgroups.class);
private final GroupControl.Factory controlFactory; private final GroupControl.Factory controlFactory;
private final GroupIncludeCache groupIncludeCache; private final GroupIncludeCache groupIncludeCache;
private final GroupJson json; private final GroupJson json;
@Inject @Inject
ListIncludedGroups( ListSubgroups(
GroupControl.Factory controlFactory, GroupIncludeCache groupIncludeCache, GroupJson json) { GroupControl.Factory controlFactory, GroupIncludeCache groupIncludeCache, GroupJson json) {
this.controlFactory = controlFactory; this.controlFactory = controlFactory;
this.groupIncludeCache = groupIncludeCache; this.groupIncludeCache = groupIncludeCache;
@ -57,19 +57,18 @@ public class ListIncludedGroups implements RestReadView<GroupResource> {
boolean ownerOfParent = rsrc.getControl().isOwner(); boolean ownerOfParent = rsrc.getControl().isOwner();
List<GroupInfo> included = new ArrayList<>(); List<GroupInfo> included = new ArrayList<>();
Collection<AccountGroup.UUID> includedGroupUuids = Collection<AccountGroup.UUID> subgroupUuids =
groupIncludeCache.subgroupsOf(group.getGroupUUID()); groupIncludeCache.subgroupsOf(group.getGroupUUID());
for (AccountGroup.UUID includedGroupUuid : includedGroupUuids) { for (AccountGroup.UUID subgroupUuid : subgroupUuids) {
try { try {
GroupControl i = controlFactory.controlFor(includedGroupUuid); GroupControl i = controlFactory.controlFor(subgroupUuid);
if (ownerOfParent || i.isVisible()) { if (ownerOfParent || i.isVisible()) {
included.add(json.format(i.getGroup())); included.add(json.format(i.getGroup()));
} }
} catch (NoSuchGroupException notFound) { } catch (NoSuchGroupException notFound) {
log.warn( log.warn(
String.format( String.format(
"Group %s no longer available, included into %s", "Group %s no longer available, subgroup of %s", subgroupUuid, group.getName()));
includedGroupUuid, group.getName()));
continue; continue;
} }
} }

View File

@ -15,18 +15,18 @@
package com.google.gerrit.server.group; package com.google.gerrit.server.group;
import static com.google.gerrit.server.group.GroupResource.GROUP_KIND; import static com.google.gerrit.server.group.GroupResource.GROUP_KIND;
import static com.google.gerrit.server.group.IncludedGroupResource.INCLUDED_GROUP_KIND;
import static com.google.gerrit.server.group.MemberResource.MEMBER_KIND; import static com.google.gerrit.server.group.MemberResource.MEMBER_KIND;
import static com.google.gerrit.server.group.SubgroupResource.SUBGROUP_KIND;
import com.google.gerrit.audit.GroupMemberAuditListener; import com.google.gerrit.audit.GroupMemberAuditListener;
import com.google.gerrit.extensions.registration.DynamicMap; import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.registration.DynamicSet; import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.restapi.RestApiModule; import com.google.gerrit.extensions.restapi.RestApiModule;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.group.AddIncludedGroups.UpdateIncludedGroup;
import com.google.gerrit.server.group.AddMembers.UpdateMember; import com.google.gerrit.server.group.AddMembers.UpdateMember;
import com.google.gerrit.server.group.DeleteIncludedGroups.DeleteIncludedGroup; import com.google.gerrit.server.group.AddSubgroups.UpdateSubgroup;
import com.google.gerrit.server.group.DeleteMembers.DeleteMember; import com.google.gerrit.server.group.DeleteMembers.DeleteMember;
import com.google.gerrit.server.group.DeleteSubgroups.DeleteSubgroup;
import com.google.inject.Provides; import com.google.inject.Provides;
public class Module extends RestApiModule { public class Module extends RestApiModule {
@ -36,7 +36,7 @@ public class Module extends RestApiModule {
DynamicMap.mapOf(binder(), GROUP_KIND); DynamicMap.mapOf(binder(), GROUP_KIND);
DynamicMap.mapOf(binder(), MEMBER_KIND); DynamicMap.mapOf(binder(), MEMBER_KIND);
DynamicMap.mapOf(binder(), INCLUDED_GROUP_KIND); DynamicMap.mapOf(binder(), SUBGROUP_KIND);
get(GROUP_KIND).to(GetGroup.class); get(GROUP_KIND).to(GetGroup.class);
put(GROUP_KIND).to(PutGroup.class); put(GROUP_KIND).to(PutGroup.class);
@ -45,9 +45,9 @@ public class Module extends RestApiModule {
post(GROUP_KIND, "members").to(AddMembers.class); post(GROUP_KIND, "members").to(AddMembers.class);
post(GROUP_KIND, "members.add").to(AddMembers.class); post(GROUP_KIND, "members.add").to(AddMembers.class);
post(GROUP_KIND, "members.delete").to(DeleteMembers.class); post(GROUP_KIND, "members.delete").to(DeleteMembers.class);
post(GROUP_KIND, "groups").to(AddIncludedGroups.class); post(GROUP_KIND, "groups").to(AddSubgroups.class);
post(GROUP_KIND, "groups.add").to(AddIncludedGroups.class); post(GROUP_KIND, "groups.add").to(AddSubgroups.class);
post(GROUP_KIND, "groups.delete").to(DeleteIncludedGroups.class); post(GROUP_KIND, "groups.delete").to(DeleteSubgroups.class);
get(GROUP_KIND, "description").to(GetDescription.class); get(GROUP_KIND, "description").to(GetDescription.class);
put(GROUP_KIND, "description").to(PutDescription.class); put(GROUP_KIND, "description").to(PutDescription.class);
delete(GROUP_KIND, "description").to(PutDescription.class); delete(GROUP_KIND, "description").to(PutDescription.class);
@ -64,10 +64,10 @@ public class Module extends RestApiModule {
put(MEMBER_KIND).to(UpdateMember.class); put(MEMBER_KIND).to(UpdateMember.class);
delete(MEMBER_KIND).to(DeleteMember.class); delete(MEMBER_KIND).to(DeleteMember.class);
child(GROUP_KIND, "groups").to(IncludedGroupsCollection.class); child(GROUP_KIND, "groups").to(SubgroupsCollection.class);
get(INCLUDED_GROUP_KIND).to(GetIncludedGroup.class); get(SUBGROUP_KIND).to(GetSubgroup.class);
put(INCLUDED_GROUP_KIND).to(UpdateIncludedGroup.class); put(SUBGROUP_KIND).to(UpdateSubgroup.class);
delete(INCLUDED_GROUP_KIND).to(DeleteIncludedGroup.class); delete(SUBGROUP_KIND).to(DeleteSubgroup.class);
factory(CreateGroup.Factory.class); factory(CreateGroup.Factory.class);
factory(GroupsUpdate.Factory.class); factory(GroupsUpdate.Factory.class);

View File

@ -19,13 +19,13 @@ import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.inject.TypeLiteral; import com.google.inject.TypeLiteral;
public class IncludedGroupResource extends GroupResource { public class SubgroupResource extends GroupResource {
public static final TypeLiteral<RestView<IncludedGroupResource>> INCLUDED_GROUP_KIND = public static final TypeLiteral<RestView<SubgroupResource>> SUBGROUP_KIND =
new TypeLiteral<RestView<IncludedGroupResource>>() {}; new TypeLiteral<RestView<SubgroupResource>>() {};
private final GroupDescription.Basic member; private final GroupDescription.Basic member;
public IncludedGroupResource(GroupResource group, GroupDescription.Basic member) { public SubgroupResource(GroupResource group, GroupDescription.Basic member) {
super(group); super(group);
this.member = member; this.member = member;
} }

View File

@ -26,36 +26,36 @@ import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestView; import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.extensions.restapi.TopLevelResource; import com.google.gerrit.extensions.restapi.TopLevelResource;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.group.AddIncludedGroups.PutIncludedGroup; import com.google.gerrit.server.group.AddSubgroups.PutSubgroup;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
import com.google.inject.Singleton; import com.google.inject.Singleton;
@Singleton @Singleton
public class IncludedGroupsCollection public class SubgroupsCollection
implements ChildCollection<GroupResource, IncludedGroupResource>, AcceptsCreate<GroupResource> { implements ChildCollection<GroupResource, SubgroupResource>, AcceptsCreate<GroupResource> {
private final DynamicMap<RestView<IncludedGroupResource>> views; private final DynamicMap<RestView<SubgroupResource>> views;
private final ListIncludedGroups list; private final ListSubgroups list;
private final GroupsCollection groupsCollection; private final GroupsCollection groupsCollection;
private final Provider<ReviewDb> dbProvider; private final Provider<ReviewDb> dbProvider;
private final Groups groups; private final Groups groups;
private final AddIncludedGroups put; private final AddSubgroups addSubgroups;
@Inject @Inject
IncludedGroupsCollection( SubgroupsCollection(
DynamicMap<RestView<IncludedGroupResource>> views, DynamicMap<RestView<SubgroupResource>> views,
ListIncludedGroups list, ListSubgroups list,
GroupsCollection groupsCollection, GroupsCollection groupsCollection,
Provider<ReviewDb> dbProvider, Provider<ReviewDb> dbProvider,
Groups groups, Groups groups,
AddIncludedGroups put) { AddSubgroups addSubgroups) {
this.views = views; this.views = views;
this.list = list; this.list = list;
this.groupsCollection = groupsCollection; this.groupsCollection = groupsCollection;
this.dbProvider = dbProvider; this.dbProvider = dbProvider;
this.groups = groups; this.groups = groups;
this.put = put; this.addSubgroups = addSubgroups;
} }
@Override @Override
@ -64,23 +64,23 @@ public class IncludedGroupsCollection
} }
@Override @Override
public IncludedGroupResource parse(GroupResource resource, IdString id) public SubgroupResource parse(GroupResource resource, IdString id)
throws MethodNotAllowedException, AuthException, ResourceNotFoundException, OrmException { throws MethodNotAllowedException, AuthException, ResourceNotFoundException, OrmException {
GroupDescription.Internal parent = GroupDescription.Internal parent =
resource.asInternalGroup().orElseThrow(MethodNotAllowedException::new); resource.asInternalGroup().orElseThrow(MethodNotAllowedException::new);
GroupDescription.Basic member = GroupDescription.Basic member =
groupsCollection.parse(TopLevelResource.INSTANCE, id).getGroup(); groupsCollection.parse(TopLevelResource.INSTANCE, id).getGroup();
if (resource.getControl().canSeeGroup() && isMember(parent, member)) { if (resource.getControl().canSeeGroup() && isSubgroup(parent, member)) {
return new IncludedGroupResource(resource, member); return new SubgroupResource(resource, member);
} }
throw new ResourceNotFoundException(id); throw new ResourceNotFoundException(id);
} }
private boolean isMember(GroupDescription.Internal parent, GroupDescription.Basic member) private boolean isSubgroup(GroupDescription.Internal parent, GroupDescription.Basic member)
throws OrmException, ResourceNotFoundException { throws OrmException, ResourceNotFoundException {
try { try {
return groups.isIncluded(dbProvider.get(), parent.getGroupUUID(), member.getGroupUUID()); return groups.isSubgroup(dbProvider.get(), parent.getGroupUUID(), member.getGroupUUID());
} catch (NoSuchGroupException e) { } catch (NoSuchGroupException e) {
throw new ResourceNotFoundException( throw new ResourceNotFoundException(
String.format("Group %s not found", parent.getGroupUUID())); String.format("Group %s not found", parent.getGroupUUID()));
@ -89,12 +89,12 @@ public class IncludedGroupsCollection
@SuppressWarnings("unchecked") @SuppressWarnings("unchecked")
@Override @Override
public PutIncludedGroup create(GroupResource group, IdString id) { public PutSubgroup create(GroupResource group, IdString id) {
return new PutIncludedGroup(put, id.get()); return new PutSubgroup(addSubgroups, id.get());
} }
@Override @Override
public DynamicMap<RestView<IncludedGroupResource>> views() { public DynamicMap<RestView<SubgroupResource>> views() {
return views; return views;
} }
} }

View File

@ -25,8 +25,8 @@ import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.TopLevelResource; import com.google.gerrit.extensions.restapi.TopLevelResource;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.server.group.AddIncludedGroups;
import com.google.gerrit.server.group.AddMembers; import com.google.gerrit.server.group.AddMembers;
import com.google.gerrit.server.group.AddSubgroups;
import com.google.gerrit.server.group.CreateGroup; import com.google.gerrit.server.group.CreateGroup;
import com.google.gerrit.server.group.GroupResource; import com.google.gerrit.server.group.GroupResource;
import com.google.gerrit.server.group.GroupsCollection; import com.google.gerrit.server.group.GroupsCollection;
@ -101,7 +101,7 @@ final class CreateGroupCommand extends SshCommand {
@Inject private AddMembers addMembers; @Inject private AddMembers addMembers;
@Inject private AddIncludedGroups addIncludedGroups; @Inject private AddSubgroups addSubgroups;
@Override @Override
protected void run() throws Failure, OrmException, IOException, ConfigInvalidException { protected void run() throws Failure, OrmException, IOException, ConfigInvalidException {
@ -113,7 +113,7 @@ final class CreateGroupCommand extends SshCommand {
} }
if (!initialGroups.isEmpty()) { if (!initialGroups.isEmpty()) {
addIncludedGroups(rsrc); addSubgroups(rsrc);
} }
} catch (RestApiException e) { } catch (RestApiException e) {
throw die(e); throw die(e);
@ -142,11 +142,10 @@ final class CreateGroupCommand extends SshCommand {
addMembers.apply(rsrc, input); addMembers.apply(rsrc, input);
} }
private void addIncludedGroups(GroupResource rsrc) private void addSubgroups(GroupResource rsrc) throws RestApiException, OrmException, IOException {
throws RestApiException, OrmException, IOException { AddSubgroups.Input input =
AddIncludedGroups.Input input = AddSubgroups.Input.fromGroups(
AddIncludedGroups.Input.fromGroups(
initialGroups.stream().map(AccountGroup.UUID::get).collect(toList())); initialGroups.stream().map(AccountGroup.UUID::get).collect(toList()));
addIncludedGroups.apply(rsrc, input); addSubgroups.apply(rsrc, input);
} }
} }

View File

@ -26,10 +26,10 @@ import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.GroupCache; import com.google.gerrit.server.account.GroupCache;
import com.google.gerrit.server.group.AddIncludedGroups;
import com.google.gerrit.server.group.AddMembers; import com.google.gerrit.server.group.AddMembers;
import com.google.gerrit.server.group.DeleteIncludedGroups; import com.google.gerrit.server.group.AddSubgroups;
import com.google.gerrit.server.group.DeleteMembers; import com.google.gerrit.server.group.DeleteMembers;
import com.google.gerrit.server.group.DeleteSubgroups;
import com.google.gerrit.server.group.GroupResource; import com.google.gerrit.server.group.GroupResource;
import com.google.gerrit.server.group.GroupsCollection; import com.google.gerrit.server.group.GroupsCollection;
import com.google.gerrit.server.group.InternalGroup; import com.google.gerrit.server.group.InternalGroup;
@ -94,9 +94,9 @@ public class SetMembersCommand extends SshCommand {
@Inject private DeleteMembers deleteMembers; @Inject private DeleteMembers deleteMembers;
@Inject private AddIncludedGroups addIncludedGroups; @Inject private AddSubgroups addSubgroups;
@Inject private DeleteIncludedGroups deleteIncludedGroups; @Inject private DeleteSubgroups deleteSubgroups;
@Inject private GroupsCollection groupsCollection; @Inject private GroupsCollection groupsCollection;
@ -115,7 +115,7 @@ public class SetMembersCommand extends SshCommand {
reportMembersAction("removed from", resource, accountsToRemove); reportMembersAction("removed from", resource, accountsToRemove);
} }
if (!groupsToRemove.isEmpty()) { if (!groupsToRemove.isEmpty()) {
deleteIncludedGroups.apply(resource, fromGroups(groupsToRemove)); deleteSubgroups.apply(resource, fromGroups(groupsToRemove));
reportGroupsAction("excluded from", resource, groupsToRemove); reportGroupsAction("excluded from", resource, groupsToRemove);
} }
if (!accountsToAdd.isEmpty()) { if (!accountsToAdd.isEmpty()) {
@ -123,7 +123,7 @@ public class SetMembersCommand extends SshCommand {
reportMembersAction("added to", resource, accountsToAdd); reportMembersAction("added to", resource, accountsToAdd);
} }
if (!groupsToInclude.isEmpty()) { if (!groupsToInclude.isEmpty()) {
addIncludedGroups.apply(resource, fromGroups(groupsToInclude)); addSubgroups.apply(resource, fromGroups(groupsToInclude));
reportGroupsAction("included to", resource, groupsToInclude); reportGroupsAction("included to", resource, groupsToInclude);
} }
} }
@ -160,9 +160,8 @@ public class SetMembersCommand extends SshCommand {
String.format("Groups %s group %s: %s\n", action, group.getName(), names).getBytes(ENC)); String.format("Groups %s group %s: %s\n", action, group.getName(), names).getBytes(ENC));
} }
private AddIncludedGroups.Input fromGroups(List<AccountGroup.UUID> accounts) { private AddSubgroups.Input fromGroups(List<AccountGroup.UUID> accounts) {
return AddIncludedGroups.Input.fromGroups( return AddSubgroups.Input.fromGroups(accounts.stream().map(Object::toString).collect(toList()));
accounts.stream().map(Object::toString).collect(toList()));
} }
private AddMembers.Input fromMembers(List<Account.Id> accounts) { private AddMembers.Input fromMembers(List<Account.Id> accounts) {