opendev/gerrit
Code Issues Proposed changes

SetMembersCommand: Handle REST errors gracefully

Browse Source 
If a REST error occurs while adding a member or group to a group, the
command fails with "internal server error" which is not helpful to the
user.

Catch REST errors and allow them to be reported back to the user.

Other types of error will still result in "internal server error".

Change-Id: Ia21f7aa28212f6d93ec4e6ff74a9de9ee6428a00
This commit is contained in:
David Pursehouse
2016-11-04 10:21:06 +09:00
parent 18d8f00aeb
commit 664a284bc6
1 changed files with 24 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

43
gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetMembersCommand.java
View File

@@ -20,6 +20,7 @@ import com.google.common.base.MoreObjects;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.TopLevelResource;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
@@ -84,26 +85,30 @@ public class SetMembersCommand extends SshCommand {
@Override
protected void run() throws UnloggedFailure, Failure, Exception {
for (AccountGroup.UUID groupUuid : groups) {
GroupResource resource =
groupsCollection.parse(TopLevelResource.INSTANCE,
IdString.fromUrl(groupUuid.get()));
if (!accountsToRemove.isEmpty()) {
deleteMembers.apply(resource, fromMembers(accountsToRemove));
reportMembersAction("removed from", resource, accountsToRemove);
}
if (!groupsToRemove.isEmpty()) {
deleteIncludedGroups.apply(resource, fromGroups(groupsToRemove));
reportGroupsAction("excluded from", resource, groupsToRemove);
}
if (!accountsToAdd.isEmpty()) {
addMembers.apply(resource, fromMembers(accountsToAdd));
reportMembersAction("added to", resource, accountsToAdd);
}
if (!groupsToInclude.isEmpty()) {
addIncludedGroups.apply(resource, fromGroups(groupsToInclude));
reportGroupsAction("included to", resource, groupsToInclude);
try {
for (AccountGroup.UUID groupUuid : groups) {
GroupResource resource =
groupsCollection.parse(TopLevelResource.INSTANCE,
IdString.fromUrl(groupUuid.get()));
if (!accountsToRemove.isEmpty()) {
deleteMembers.apply(resource, fromMembers(accountsToRemove));
reportMembersAction("removed from", resource, accountsToRemove);
}
if (!groupsToRemove.isEmpty()) {
deleteIncludedGroups.apply(resource, fromGroups(groupsToRemove));
reportGroupsAction("excluded from", resource, groupsToRemove);
}
if (!accountsToAdd.isEmpty()) {
addMembers.apply(resource, fromMembers(accountsToAdd));
reportMembersAction("added to", resource, accountsToAdd);
}
if (!groupsToInclude.isEmpty()) {
addIncludedGroups.apply(resource, fromGroups(groupsToInclude));
reportGroupsAction("included to", resource, groupsToInclude);
}
}
} catch (RestApiException e) {
throw die(e.getMessage());
}
}