From 3adce619c9799faa8f88c96e6d8f685c4132c9ae Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Mon, 14 Jan 2013 14:22:03 +0100 Subject: [PATCH] Add REST endpoint to remove members from a group With DELETE on 'groups/*/members/' it is now possible to remove a member from a group. In addition with DELETE on 'groups/*/members' it is possible to remove multiple members from a group at once. The AccountGroupMembersScreen uses the new REST endpoint to remove members. Change-Id: I13036333f1c23fa0c33e09ddf0cec27e95265b9d Signed-off-by: Edwin Kempin --- .../com/google/gerrit/client/VoidResult.java | 22 +++ .../admin/AccountGroupMembersScreen.java | 13 +- .../google/gerrit/client/groups/GroupApi.java | 13 ++ .../gerrit/server/group/DeleteMembers.java | 176 ++++++++++++++++++ .../gerrit/server/group/MemberResource.java | 9 +- .../server/group/MembersCollection.java | 3 +- .../google/gerrit/server/group/Module.java | 3 + 7 files changed, 230 insertions(+), 9 deletions(-) create mode 100644 gerrit-gwtui/src/main/java/com/google/gerrit/client/VoidResult.java create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/group/DeleteMembers.java diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/VoidResult.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/VoidResult.java new file mode 100644 index 0000000000..1f7418d129 --- /dev/null +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/VoidResult.java @@ -0,0 +1,22 @@ +// Copyright (C) 2013 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.client; + +import com.google.gwt.core.client.JavaScriptObject; + +public final class VoidResult extends JavaScriptObject { + protected VoidResult() { + } +} diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AccountGroupMembersScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AccountGroupMembersScreen.java index 78b8d0870f..7184a48679 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AccountGroupMembersScreen.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AccountGroupMembersScreen.java @@ -241,21 +241,20 @@ public class AccountGroupMembersScreen extends AccountGroupScreen { } void deleteChecked() { - final HashSet ids = - new HashSet(); + final HashSet ids = new HashSet(); for (int row = 1; row < table.getRowCount(); row++) { final AccountGroupMember k = getRowItem(row); if (k != null && ((CheckBox) table.getWidget(row, 1)).getValue()) { - ids.add(k.getKey()); + ids.add(k.getAccountId()); } } if (!ids.isEmpty()) { - Util.GROUP_SVC.deleteGroupMembers(getGroupId(), ids, - new GerritCallback() { - public void onSuccess(final VoidResult result) { + GroupApi.removeMembers(getGroupUUID(), ids, + new GerritCallback() { + public void onSuccess(final com.google.gerrit.client.VoidResult result) { for (int row = 1; row < table.getRowCount();) { final AccountGroupMember k = getRowItem(row); - if (k != null && ids.contains(k.getKey())) { + if (k != null && ids.contains(k.getAccountId())) { table.removeRow(row); } else { row++; diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/groups/GroupApi.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/groups/GroupApi.java index a2220486b6..ac7e67ed10 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/groups/GroupApi.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/groups/GroupApi.java @@ -14,8 +14,10 @@ package com.google.gerrit.client.groups; +import com.google.gerrit.client.VoidResult; import com.google.gerrit.client.rpc.NativeList; import com.google.gerrit.client.rpc.RestApi; +import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gwt.core.client.JavaScriptObject; import com.google.gwt.http.client.URL; @@ -46,6 +48,17 @@ public class GroupApi { call.data(input).put(cb); } + /** Remove members from a group. */ + public static void removeMembers(AccountGroup.UUID groupUUID, + Set ids, AsyncCallback cb) { + RestApi call = new RestApi(membersBase(groupUUID)); + MemberInput input = MemberInput.create(); + for (Account.Id id : ids) { + input.add_member(id.toString()); + } + call.data(input).delete(cb); + } + private static String membersBase(AccountGroup.UUID groupUUID) { return base(groupUUID) + "members"; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/DeleteMembers.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/DeleteMembers.java new file mode 100644 index 0000000000..db03fd1271 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/DeleteMembers.java @@ -0,0 +1,176 @@ +// Copyright (C) 2013 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.server.group; + +import com.google.common.collect.Lists; +import com.google.common.collect.Maps; +import com.google.gerrit.common.data.GroupDescription; +import com.google.gerrit.common.errors.NoSuchAccountException; +import com.google.gerrit.common.errors.NoSuchGroupException; +import com.google.gerrit.extensions.restapi.AuthException; +import com.google.gerrit.extensions.restapi.BadRequestException; +import com.google.gerrit.extensions.restapi.MethodNotAllowedException; +import com.google.gerrit.extensions.restapi.Response; +import com.google.gerrit.extensions.restapi.RestModifyView; +import com.google.gerrit.reviewdb.client.Account; +import com.google.gerrit.reviewdb.client.AccountGroup; +import com.google.gerrit.reviewdb.client.AccountGroupMember; +import com.google.gerrit.reviewdb.client.AccountGroupMemberAudit; +import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.server.BadRequestHandler; +import com.google.gerrit.server.CurrentUser; +import com.google.gerrit.server.IdentifiedUser; +import com.google.gerrit.server.account.AccountCache; +import com.google.gerrit.server.account.AccountResolver; +import com.google.gerrit.server.account.GroupControl; +import com.google.gerrit.server.group.PutMembers.Input; +import com.google.gwtorm.server.OrmException; +import com.google.inject.Inject; +import com.google.inject.Provider; + +import java.util.List; +import java.util.Map; + +public class DeleteMembers implements RestModifyView { + private final GroupControl.Factory groupControlFactory; + private final AccountResolver accountResolver; + private final AccountCache accountCache; + private final ReviewDb db; + private final Provider self; + + @Inject + DeleteMembers(final GroupControl.Factory groupControlFactory, + final AccountResolver accountResolver, final AccountCache accountCache, + final ReviewDb db, final Provider self) { + this.groupControlFactory = groupControlFactory; + this.accountResolver = accountResolver; + this.accountCache = accountCache; + this.db = db; + this.self = self; + } + + @Override + public Class inputType() { + return Input.class; + } + + @Override + public Object apply(GroupResource resource, Input input) + throws AuthException, MethodNotAllowedException, BadRequestException, + OrmException, NoSuchGroupException { + final GroupDescription.Basic group = resource.getGroup(); + if (!(group instanceof GroupDescription.Internal)) { + throw new MethodNotAllowedException(); + } + + input = Input.init(input); + + final AccountGroup internalGroup = ((GroupDescription.Internal) group).getAccountGroup(); + final GroupControl control = groupControlFactory.controlFor(internalGroup); + final Map members = getMembers(internalGroup.getId()); + final List toRemove = Lists.newLinkedList(); + final BadRequestHandler badRequest = new BadRequestHandler("removing group members"); + + for (final String nameOrEmail : input.members) { + Account a = accountResolver.find(nameOrEmail); + if (a == null) { + badRequest.addError(new NoSuchAccountException(nameOrEmail)); + continue; + } + + if (!control.canRemoveMember(a.getId())) { + throw new AuthException("Cannot delete member: " + a.getFullName()); + } + + final AccountGroupMember m = members.remove(a.getId()); + if (m != null) { + toRemove.add(m); + } + } + + badRequest.failOnError(); + + writeAudits(toRemove); + db.accountGroupMembers().delete(toRemove); + for (final AccountGroupMember m : toRemove) { + accountCache.evict(m.getAccountId()); + } + + return Response.none(); + } + + private void writeAudits(final List toBeRemoved) + throws OrmException { + final Account.Id me = ((IdentifiedUser) self.get()).getAccountId(); + final List auditUpdates = Lists.newLinkedList(); + final List auditInserts = Lists.newLinkedList(); + for (final AccountGroupMember m : toBeRemoved) { + AccountGroupMemberAudit audit = null; + for (AccountGroupMemberAudit a : db.accountGroupMembersAudit() + .byGroupAccount(m.getAccountGroupId(), m.getAccountId())) { + if (a.isActive()) { + audit = a; + break; + } + } + + if (audit != null) { + audit.removed(me); + auditUpdates.add(audit); + } else { + audit = new AccountGroupMemberAudit(m, me); + audit.removedLegacy(); + auditInserts.add(audit); + } + } + db.accountGroupMembersAudit().update(auditUpdates); + db.accountGroupMembersAudit().insert(auditInserts); + } + + private Map getMembers( + final AccountGroup.Id groupId) throws OrmException, NoSuchGroupException { + final Map members = Maps.newHashMap(); + for (final AccountGroupMember m : db.accountGroupMembers().byGroup(groupId)) { + members.put(m.getAccountId(), m); + } + return members; + } + + static class DeleteMember implements RestModifyView { + static class Input { + } + + private final Provider delete; + + @Inject + DeleteMember(final Provider delete) { + this.delete = delete; + } + + @Override + public Class inputType() { + return DeleteMember.Input.class; + } + + @Override + public Object apply(MemberResource resource, Input input) + throws AuthException, MethodNotAllowedException, BadRequestException, + OrmException, NoSuchGroupException { + PutMembers.Input in = new PutMembers.Input(); + in._oneMember = resource.getUser().getAccountId().toString(); + return delete.get().apply(resource.getGroup(), in); + } + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/MemberResource.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/MemberResource.java index 6249fc148f..a121e98df5 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/MemberResource.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/MemberResource.java @@ -23,7 +23,14 @@ public class MemberResource extends AccountResource { public static final TypeLiteral> MEMBER_KIND = new TypeLiteral>() {}; - public MemberResource(IdentifiedUser user) { + private final GroupResource group; + + public MemberResource(GroupResource group, IdentifiedUser user) { super(user); + this.group = group; + } + + public GroupResource getGroup() { + return group; } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/MembersCollection.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/MembersCollection.java index 003f54e67e..f4006faecb 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/MembersCollection.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/MembersCollection.java @@ -83,7 +83,8 @@ public class MembersCollection implements if (groupDetail.members != null) { for (final AccountGroupMember member : groupDetail.members) { if (member.getAccountId().equals(a.getId())) { - return new MemberResource(userGenericFactory.create(a.getId())); + return new MemberResource(parent, + userGenericFactory.create(a.getId())); } } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/Module.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/Module.java index 30401c2832..14bb2da25e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/Module.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/Module.java @@ -20,6 +20,7 @@ import static com.google.gerrit.server.group.MemberResource.MEMBER_KIND; import com.google.gerrit.extensions.registration.DynamicMap; import com.google.gerrit.extensions.restapi.RestApiModule; +import com.google.gerrit.server.group.DeleteMembers.DeleteMember; public class Module extends RestApiModule { @Override @@ -35,6 +36,8 @@ public class Module extends RestApiModule { child(GROUP_KIND, "members").to(MembersCollection.class); get(MEMBER_KIND).to(GetMember.class); put(GROUP_KIND, "members").to(PutMembers.class); + delete(GROUP_KIND, "members").to(DeleteMembers.class); + delete(MEMBER_KIND).to(DeleteMember.class); child(GROUP_KIND, "groups").to(IncludedGroupsCollection.class); get(INCLUDED_GROUP_KIND).to(GetIncludedGroup.class);