Merge "Return 422 if entity from request body cannot be processed"

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/group/GroupPropertiesIT.java

@@ -195,7 +195,7 @@ public class GroupPropertiesIT extends AbstractDaemonTest {
     in = new GroupOwnerInput();
     in.owner = "Non-Existing Group";
     r = session.put(url, in);
-    assertEquals(HttpStatus.SC_BAD_REQUEST, r.getStatusCode());
+    assertEquals(HttpStatus.SC_UNPROCESSABLE_ENTITY, r.getStatusCode());
     r.consume();
   }
 

gerrit-server/src/main/java/com/google/gerrit/server/BadRequestHandler.java

@@ -1,57 +0,0 @@
-// 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;
-
-import com.google.common.collect.Lists;
-import com.google.gerrit.extensions.restapi.BadRequestException;
-
-import java.util.List;
-
-public class BadRequestHandler {
-
-  private final List<String> errors = Lists.newLinkedList();
-  private String action;
-
-  public BadRequestHandler(final String action) {
-    this.action = action;
-  }
-
-  public void addError(final String message) {
-    errors.add(message);
-  }
-
-  public void addError(final Throwable t) {
-    errors.add(t.getMessage());
-  }
-
-  public void failOnError()
-      throws BadRequestException {
-    if (errors.isEmpty()) {
-      return;
-    }
-
-    if (errors.size() == 1) {
-      throw new BadRequestException(action + " failed: " + errors.get(0));
-    }
-
-    final StringBuilder b = new StringBuilder();
-    b.append("Multiple errors on " + action + ":");
-    for (final String error : errors) {
-      b.append("\n");
-      b.append(error);
-    }
-    throw new BadRequestException(b.toString());
-  }
-}

gerrit-server/src/main/java/com/google/gerrit/server/account/AccountsCollection.java

@@ -22,6 +22,7 @@ import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestCollection;
 import com.google.gerrit.extensions.restapi.RestView;
 import com.google.gerrit.extensions.restapi.TopLevelResource;
+import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.server.AnonymousUser;
 import com.google.gerrit.server.CurrentUser;
@@ -56,11 +57,35 @@ public class AccountsCollection implements
   @Override
   public AccountResource parse(TopLevelResource root, IdString id)
       throws ResourceNotFoundException, AuthException, OrmException {
-    return new AccountResource(parse(id.get()));
+    IdentifiedUser user = _parse(id.get());
+    if (user == null) {
+      throw new ResourceNotFoundException(id);
+    }
+    return new AccountResource(user);
   }
 
+  /**
+   * Parses a account ID from a request body and returns the user.
+   *
+   * @param id ID of the account, can be a string of the format
+   *        "Full Name <email@example.com>", just the email address, a full name
+   *        if it is unique, an account ID, a user name or 'self' for the
+   *        calling user
+   * @return the project
+   * @throws UnprocessableEntityException thrown if the account ID cannot be
+   *         resolved or if the account is not visible to the calling user
+   */
   public IdentifiedUser parse(String id) throws AuthException,
-      ResourceNotFoundException, OrmException {
+      UnprocessableEntityException, OrmException {
+    IdentifiedUser user = _parse(id);
+    if (user == null) {
+      throw new UnprocessableEntityException(String.format(
+          "Account Not Found: %s", id));
+    }
+    return user;
+  }
+
+  private IdentifiedUser _parse(String id) throws AuthException, OrmException {
     CurrentUser user = self.get();
 
     if (id.equals("self")) {
@@ -69,13 +94,13 @@ public class AccountsCollection implements
       } else if (user instanceof AnonymousUser) {
         throw new AuthException("Authentication required");
       } else {
-        throw new ResourceNotFoundException(id);
+        return null;
       }
     }
 
     Set<Account.Id> matches = resolver.findAll(id);
     if (matches.size() != 1) {
-      throw new ResourceNotFoundException(id);
+      return null;
     }
 
     Account.Id a = Iterables.getOnlyElement(matches);
@@ -83,7 +108,7 @@ public class AccountsCollection implements
         || user.getCapabilities().canAdministrateServer()) {
       return userFactory.create(a);
     } else {
-      throw new ResourceNotFoundException(id);
+      return null;
     }
   }
 

gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java

@@ -22,7 +22,6 @@ import com.google.common.collect.Lists;
 import com.google.common.collect.Sets;
 import com.google.gerrit.common.ChangeHooks;
 import com.google.gerrit.common.data.GroupDescription;
-import com.google.gerrit.common.data.GroupDescriptions;
 import com.google.gerrit.common.data.LabelTypes;
 import com.google.gerrit.common.errors.EmailException;
 import com.google.gerrit.common.errors.NoSuchGroupException;
@@ -31,6 +30,7 @@ import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.DefaultInput;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.reviewdb.client.ApprovalCategory;
@@ -126,7 +126,7 @@ public class PostReviewers implements RestModifyView<ChangeResource, Input> {
   @Override
   public PostResult apply(ChangeResource rsrc, Input input)
       throws BadRequestException, ResourceNotFoundException, AuthException,
-      OrmException, EmailException {
+      UnprocessableEntityException, OrmException, EmailException {
     if (input.reviewer == null) {
       throw new BadRequestException("missing reviewer field");
     }
@@ -139,10 +139,6 @@ public class PostReviewers implements RestModifyView<ChangeResource, Input> {
       }
     } catch (NoSuchChangeException e) {
       throw new ResourceNotFoundException(e.getMessage());
-    } catch (NoSuchGroupException e) {
-      throw new ResourceNotFoundException(e.getMessage());
-    } catch (NoSuchProjectException e) {
-      throw new ResourceNotFoundException(e.getMessage());
     }
   }
 
@@ -154,13 +150,10 @@ public class PostReviewers implements RestModifyView<ChangeResource, Input> {
   }
 
   private PostResult putGroup(ChangeResource rsrc, Input input)
-      throws ResourceNotFoundException, AuthException, NoSuchGroupException,
+      throws ResourceNotFoundException, AuthException, BadRequestException,
-      NoSuchProjectException, OrmException, NoSuchChangeException,
+      UnprocessableEntityException, OrmException, NoSuchChangeException,
       EmailException {
-    GroupDescription.Basic group = groupsCollection.get().parse(input.reviewer);
+    GroupDescription.Basic group = groupsCollection.get().parseInternal(input.reviewer);
-    if (GroupDescriptions.toAccountGroup(group) == null) {
-      throw new ResourceNotFoundException(input.reviewer);
-    }
     PostResult result = new PostResult();
     if (!isLegalReviewerGroup(group.getGroupUUID())) {
       result.error = MessageFormat.format(
@@ -170,8 +163,15 @@ public class PostReviewers implements RestModifyView<ChangeResource, Input> {
 
     Set<IdentifiedUser> reviewers = Sets.newLinkedHashSet();
     ChangeControl control = rsrc.getControl();
-    Set<Account> members = groupMembersFactory.create(control.getCurrentUser())
+    Set<Account> members;
-        .listAccounts(group.getGroupUUID(), control.getProject().getNameKey());
+    try {
+      members = groupMembersFactory.create(control.getCurrentUser()).listAccounts(
+              group.getGroupUUID(), control.getProject().getNameKey());
+    } catch (NoSuchGroupException e) {
+      throw new UnprocessableEntityException(e.getMessage());
+    } catch (NoSuchProjectException e) {
+      throw new BadRequestException(e.getMessage());
+    }
 
     // if maxAllowed is set to 0, it is allowed to add any number of
     // reviewers

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

@@ -19,19 +19,17 @@ import com.google.common.collect.ImmutableList;
 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.NoSuchGroupException;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.DefaultInput;
 import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
-import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.reviewdb.client.AccountGroupIncludeByUuid;
 import com.google.gerrit.reviewdb.client.AccountGroupIncludeByUuidAudit;
 import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.BadRequestHandler;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.account.GroupControl;
 import com.google.gerrit.server.account.GroupIncludeCache;
@@ -83,7 +81,7 @@ public class AddIncludedGroups implements RestModifyView<GroupResource, Input> {
   @Override
   public List<GroupInfo> apply(GroupResource resource, Input input)
       throws MethodNotAllowedException, AuthException, BadRequestException,
-      OrmException {
+      UnprocessableEntityException, OrmException {
     AccountGroup group = resource.toAccountGroup();
     if (group == null) {
       throw new MethodNotAllowedException();
@@ -93,19 +91,11 @@ public class AddIncludedGroups implements RestModifyView<GroupResource, Input> {
     GroupControl control = resource.getControl();
     Map<AccountGroup.UUID, AccountGroupIncludeByUuid> newIncludedGroups = Maps.newHashMap();
     List<AccountGroupIncludeByUuidAudit> newIncludedGroupsAudits = Lists.newLinkedList();
-    BadRequestHandler badRequest = new BadRequestHandler("adding groups");
     List<GroupInfo> result = Lists.newLinkedList();
     Account.Id me = ((IdentifiedUser) control.getCurrentUser()).getAccountId();
 
     for (String includedGroup : input.groups) {
-      GroupDescription.Basic d;
+      GroupDescription.Basic d = groupsCollection.get().parse(includedGroup);
-      try {
-        d = groupsCollection.get().parse(includedGroup);
-      } catch (ResourceNotFoundException e) {
-        badRequest.addError(new NoSuchGroupException(includedGroup));
-        continue;
-      }
-
       if (!control.canAddGroup(d.getGroupUUID())) {
         throw new AuthException(String.format("Cannot add group: %s",
             d.getName()));
@@ -125,8 +115,6 @@ public class AddIncludedGroups implements RestModifyView<GroupResource, Input> {
       result.add(json.format(d));
     }
 
-    badRequest.failOnError();
-
     if (!newIncludedGroups.isEmpty()) {
       db.accountGroupIncludesByUuidAudit().insert(newIncludedGroupsAudits);
       db.accountGroupIncludesByUuid().insert(newIncludedGroups.values());
@@ -154,7 +142,7 @@ public class AddIncludedGroups implements RestModifyView<GroupResource, Input> {
     @Override
     public GroupInfo apply(GroupResource resource, Input input)
         throws MethodNotAllowedException, AuthException, BadRequestException,
-        OrmException {
+        UnprocessableEntityException, OrmException {
       AddIncludedGroups.Input in = new AddIncludedGroups.Input();
       in.groups = ImmutableList.of(id);
       List<GroupInfo> list = put.get().apply(resource, in);

gerrit-server/src/main/java/com/google/gerrit/server/group/AddMembers.java

@@ -17,21 +17,17 @@ package com.google.gerrit.server.group;
 import com.google.common.base.Strings;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
-import com.google.gerrit.common.errors.InactiveAccountException;
-import com.google.gerrit.common.errors.NoSuchAccountException;
 import com.google.gerrit.extensions.restapi.AuthException;
-import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.DefaultInput;
 import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
-import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
 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.client.AuthType;
 import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.BadRequestHandler;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.account.AccountCache;
 import com.google.gerrit.server.account.AccountException;
@@ -95,8 +91,8 @@ class AddMembers implements RestModifyView<GroupResource, Input> {
 
   @Override
   public List<AccountInfo> apply(GroupResource resource, Input input)
-      throws AuthException, MethodNotAllowedException, BadRequestException,
+      throws AuthException, MethodNotAllowedException,
-      OrmException {
+      UnprocessableEntityException, OrmException {
     AccountGroup internalGroup = resource.toAccountGroup();
     if (internalGroup == null) {
       throw new MethodNotAllowedException();
@@ -106,20 +102,14 @@ class AddMembers implements RestModifyView<GroupResource, Input> {
     GroupControl control = resource.getControl();
     Map<Account.Id, AccountGroupMember> newAccountGroupMembers = Maps.newHashMap();
     List<AccountGroupMemberAudit> newAccountGroupMemberAudits = Lists.newLinkedList();
-    BadRequestHandler badRequest = new BadRequestHandler("adding new group members");
     List<AccountInfo> result = Lists.newLinkedList();
     Account.Id me = ((IdentifiedUser) control.getCurrentUser()).getAccountId();
 
     for (String nameOrEmail : input.members) {
       Account a = findAccount(nameOrEmail);
-      if (a == null) {
-        badRequest.addError(new NoSuchAccountException(nameOrEmail));
-        continue;
-      }
-
       if (!a.isActive()) {
-        badRequest.addError(new InactiveAccountException(a.getFullName()));
+        throw new UnprocessableEntityException(String.format(
-        continue;
+            "Account Inactive: %s", nameOrEmail));
       }
 
       if (!control.canAddMember(a.getId())) {
@@ -139,8 +129,6 @@ class AddMembers implements RestModifyView<GroupResource, Input> {
       result.add(AccountInfo.parse(a, true));
     }
 
-    badRequest.failOnError();
-
     db.accountGroupMembersAudit().insert(newAccountGroupMemberAudits);
     db.accountGroupMembers().insert(newAccountGroupMembers.values());
     for (AccountGroupMember m : newAccountGroupMembers.values()) {
@@ -150,13 +138,13 @@ class AddMembers implements RestModifyView<GroupResource, Input> {
     return result;
   }
 
-  private Account findAccount(String nameOrEmail) throws OrmException {
+  private Account findAccount(String nameOrEmail) throws AuthException,
+      UnprocessableEntityException, OrmException {
     try {
       return accounts.get().parse(nameOrEmail).getAccount();
-    } catch (AuthException e) {
+    } catch (UnprocessableEntityException e) {
-      return null;
+      // might be because the account does not exist or because the account is
-    } catch (ResourceNotFoundException e) {
+      // not visible
-      // might be because the account does not exist or because the account is not visible
       switch (authType) {
         case HTTP_LDAP:
         case CLIENT_SSL_CERT_LDAP:
@@ -168,7 +156,7 @@ class AddMembers implements RestModifyView<GroupResource, Input> {
           break;
         default:
       }
-      return null;
+      throw e;
     }
   }
 
@@ -201,8 +189,8 @@ class AddMembers implements RestModifyView<GroupResource, Input> {
 
     @Override
     public Object apply(GroupResource resource, PutMember.Input input)
-        throws AuthException, MethodNotAllowedException, BadRequestException,
+        throws AuthException, MethodNotAllowedException,
-        OrmException {
+        UnprocessableEntityException, OrmException {
       AddMembers.Input in = new AddMembers.Input();
       in._oneMember = id;
       List<AccountInfo> list = put.get().apply(resource, in);

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

@@ -24,9 +24,9 @@ import com.google.gerrit.common.errors.PermissionDeniedException;
 import com.google.gerrit.extensions.annotations.RequiresCapability;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.BadRequestException;
-import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestModifyView;
 import com.google.gerrit.extensions.restapi.TopLevelResource;
+import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
 import com.google.gerrit.extensions.restapi.Url;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountGroup;
@@ -78,7 +78,7 @@ class CreateGroup implements RestModifyView<TopLevelResource, Input> {
 
   @Override
   public GroupInfo apply(TopLevelResource resource, Input input)
-      throws AuthException, BadRequestException,
+      throws AuthException, BadRequestException, UnprocessableEntityException,
       NameAlreadyUsedException, OrmException {
     if (input == null) {
       input = new Input();
@@ -105,18 +105,11 @@ class CreateGroup implements RestModifyView<TopLevelResource, Input> {
     return json.format(GroupDescriptions.forAccountGroup(group));
   }
 
-  private AccountGroup.Id owner(Input input) throws BadRequestException {
+  private AccountGroup.Id owner(Input input)
+      throws UnprocessableEntityException {
     if (input.ownerId != null) {
-      try {
+      GroupDescription.Basic d = groups.parseInternal(Url.decode(input.ownerId));
-        GroupDescription.Basic d = groups.parse(Url.decode(input.ownerId));
+      return GroupDescriptions.toAccountGroup(d).getId();
-        AccountGroup owner = GroupDescriptions.toAccountGroup(d);
-        if (owner == null) {
-          throw new BadRequestException("ownerId must be internal group");
-        }
-        return owner.getId();
-      } catch (ResourceNotFoundException e) {
-        throw new BadRequestException("ownerId cannot be resolved");
-      }
     }
     return null;
   }

gerrit-server/src/main/java/com/google/gerrit/server/group/DeleteIncludedGroups.java

@@ -18,19 +18,17 @@ import com.google.common.collect.ImmutableList;
 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.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.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.Response;
 import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.reviewdb.client.AccountGroupIncludeByUuid;
 import com.google.gerrit.reviewdb.client.AccountGroupIncludeByUuidAudit;
 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.GroupControl;
@@ -62,7 +60,7 @@ public class DeleteIncludedGroups implements RestModifyView<GroupResource, Input
   @Override
   public Object apply(GroupResource resource, Input input)
       throws MethodNotAllowedException, AuthException, BadRequestException,
-      OrmException {
+      UnprocessableEntityException, OrmException {
     AccountGroup internalGroup = resource.toAccountGroup();
     if (internalGroup == null) {
       throw new MethodNotAllowedException();
@@ -72,17 +70,9 @@ public class DeleteIncludedGroups implements RestModifyView<GroupResource, Input
     final GroupControl control = resource.getControl();
     final Map<AccountGroup.UUID, AccountGroupIncludeByUuid> includedGroups = getIncludedGroups(internalGroup.getId());
     final List<AccountGroupIncludeByUuid> toRemove = Lists.newLinkedList();
-    final BadRequestHandler badRequest = new BadRequestHandler("removing included groups");
 
     for (final String includedGroup : input.groups) {
-      GroupDescription.Basic d;
+      GroupDescription.Basic d = groupsCollection.get().parse(includedGroup);
-      try {
-        d = groupsCollection.get().parse(includedGroup);
-      } catch (ResourceNotFoundException e) {
-        badRequest.addError(new NoSuchGroupException(includedGroup));
-        continue;
-      }
-
       if (!control.canRemoveGroup(d.getGroupUUID())) {
         throw new AuthException(String.format("Cannot delete group: %s",
             d.getName()));
@@ -93,7 +83,6 @@ public class DeleteIncludedGroups implements RestModifyView<GroupResource, Input
         toRemove.add(g);
       }
     }
-    badRequest.failOnError();
 
     if (!toRemove.isEmpty()) {
       writeAudits(toRemove);
@@ -155,7 +144,7 @@ public class DeleteIncludedGroups implements RestModifyView<GroupResource, Input
     @Override
     public Object apply(IncludedGroupResource resource, Input input)
         throws MethodNotAllowedException, AuthException, BadRequestException,
-        OrmException {
+        UnprocessableEntityException, OrmException {
       AddIncludedGroups.Input in = new AddIncludedGroups.Input();
       in.groups = ImmutableList.of(resource.getMember().get());
       return delete.get().apply(resource, in);

gerrit-server/src/main/java/com/google/gerrit/server/group/DeleteMembers.java

@@ -16,20 +16,17 @@ package com.google.gerrit.server.group;
 
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
-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.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.Response;
 import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
 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;
@@ -60,8 +57,8 @@ public class DeleteMembers implements RestModifyView<GroupResource, Input> {
 
   @Override
   public Object apply(GroupResource resource, Input input)
-      throws AuthException, MethodNotAllowedException, BadRequestException,
+      throws AuthException, MethodNotAllowedException,
-      OrmException {
+      UnprocessableEntityException, OrmException {
     AccountGroup internalGroup = resource.toAccountGroup();
     if (internalGroup == null) {
       throw new MethodNotAllowedException();
@@ -71,14 +68,9 @@ public class DeleteMembers implements RestModifyView<GroupResource, Input> {
     final GroupControl control = resource.getControl();
     final Map<Account.Id, AccountGroupMember> members = getMembers(internalGroup.getId());
     final List<AccountGroupMember> toRemove = Lists.newLinkedList();
-    final BadRequestHandler badRequest = new BadRequestHandler("removing group members");
 
     for (final String nameOrEmail : input.members) {
-      Account a = parse(nameOrEmail);
+      Account a = accounts.get().parse(nameOrEmail).getAccount();
-      if (a == null) {
-        badRequest.addError(new NoSuchAccountException(nameOrEmail));
-        continue;
-      }
 
       if (!control.canRemoveMember(a.getId())) {
         throw new AuthException("Cannot delete member: " + a.getFullName());
@@ -90,8 +82,6 @@ public class DeleteMembers implements RestModifyView<GroupResource, Input> {
       }
     }
 
-    badRequest.failOnError();
-
     writeAudits(toRemove);
     db.accountGroupMembers().delete(toRemove);
     for (final AccountGroupMember m : toRemove) {
@@ -101,16 +91,6 @@ public class DeleteMembers implements RestModifyView<GroupResource, Input> {
     return Response.none();
   }
 
-  private Account parse(String nameOrEmail) throws OrmException {
-    try {
-      return accounts.get().parse(nameOrEmail).getAccount();
-    } catch (AuthException e) {
-      return null;
-    } catch (ResourceNotFoundException e) {
-      return null;
-    }
-  }
-
   private void writeAudits(final List<AccountGroupMember> toBeRemoved)
       throws OrmException {
     final Account.Id me = ((IdentifiedUser) self.get()).getAccountId();
@@ -161,8 +141,8 @@ public class DeleteMembers implements RestModifyView<GroupResource, Input> {
 
     @Override
     public Object apply(MemberResource resource, Input input)
-        throws AuthException, MethodNotAllowedException, BadRequestException,
+        throws AuthException, MethodNotAllowedException,
-        OrmException, NoSuchGroupException {
+        UnprocessableEntityException, OrmException, NoSuchGroupException {
       AddMembers.Input in = new AddMembers.Input();
       in._oneMember = resource.getMember().getAccountId().toString();
       return delete.get().apply(resource, in);

gerrit-server/src/main/java/com/google/gerrit/server/group/GroupsCollection.java

@@ -15,6 +15,7 @@
 package com.google.gerrit.server.group;
 
 import com.google.gerrit.common.data.GroupDescription;
+import com.google.gerrit.common.data.GroupDescriptions;
 import com.google.gerrit.common.data.GroupReference;
 import com.google.gerrit.common.errors.NoSuchGroupException;
 import com.google.gerrit.extensions.registration.DynamicMap;
@@ -25,6 +26,7 @@ import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestCollection;
 import com.google.gerrit.extensions.restapi.RestView;
 import com.google.gerrit.extensions.restapi.TopLevelResource;
+import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
 import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.server.AnonymousUser;
 import com.google.gerrit.server.CurrentUser;
@@ -75,7 +77,7 @@ public class GroupsCollection implements
 
   @Override
   public GroupResource parse(TopLevelResource parent, IdString id)
-      throws ResourceNotFoundException, Exception {
+      throws AuthException, ResourceNotFoundException {
     final CurrentUser user = self.get();
     if (user instanceof AnonymousUser) {
       throw new AuthException("Authentication required");
@@ -83,14 +85,58 @@ public class GroupsCollection implements
       throw new ResourceNotFoundException(id);
     }
 
-    GroupControl ctl = groupControlFactory.controlFor(parse(id.get()));
+    GroupDescription.Basic group = _parse(id.get());
+    if (group == null) {
+      throw new ResourceNotFoundException(id.get());
+    }
+    GroupControl ctl = groupControlFactory.controlFor(group);
     if (!ctl.isVisible()) {
       throw new ResourceNotFoundException(id);
     }
     return new GroupResource(ctl);
   }
 
-  public GroupDescription.Basic parse(String id) throws ResourceNotFoundException {
+  /**
+   * Parses a group ID from a request body and returns the group.
+   *
+   * @param id ID of the group, can be a group UUID, a group name or a legacy
+   *        group ID
+   * @return the group
+   * @throws UnprocessableEntityException thrown if the group ID cannot be
+   *         resolved or if the group is not visible to the calling user
+   */
+  public GroupDescription.Basic parse(String id)
+      throws UnprocessableEntityException {
+    GroupDescription.Basic group = _parse(id);
+    if (group == null || !groupControlFactory.controlFor(group).isVisible()) {
+      throw new UnprocessableEntityException(String.format(
+          "Group Not Found: %s", id));
+    }
+    return group;
+  }
+
+  /**
+   * Parses a group ID from a request body and returns the group if it is a
+   * Gerrit internal group.
+   *
+   * @param id ID of the group, can be a group UUID, a group name or a legacy
+   *        group ID
+   * @return the group
+   * @throws UnprocessableEntityException thrown if the group ID cannot be
+   *         resolved, if the group is not visible to the calling user or if
+   *         it's an external group
+   */
+  public GroupDescription.Basic parseInternal(String id)
+      throws UnprocessableEntityException {
+    GroupDescription.Basic group = parse(id);
+    if (GroupDescriptions.toAccountGroup(group) == null) {
+      throw new UnprocessableEntityException(String.format(
+          "External Group Not Allowed: %s", id));
+    }
+    return group;
+  }
+
+  private GroupDescription.Basic _parse(String id) {
     AccountGroup.UUID uuid = new AccountGroup.UUID(id);
     if (groupBackend.handles(uuid)) {
       GroupDescription.Basic d = groupBackend.get(uuid);
@@ -118,7 +164,7 @@ public class GroupsCollection implements
       }
     }
 
-    throw new ResourceNotFoundException(id);
+    return null;
   }
 
   @SuppressWarnings("unchecked")

gerrit-server/src/main/java/com/google/gerrit/server/group/IncludedGroupsCollection.java

@@ -17,10 +17,12 @@ package com.google.gerrit.server.group;
 import com.google.gerrit.common.data.GroupDescription;
 import com.google.gerrit.extensions.registration.DynamicMap;
 import com.google.gerrit.extensions.restapi.AcceptsCreate;
+import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.ChildCollection;
 import com.google.gerrit.extensions.restapi.IdString;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestView;
+import com.google.gerrit.extensions.restapi.TopLevelResource;
 import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.reviewdb.client.AccountGroupIncludeByUuid;
 import com.google.gerrit.reviewdb.server.ReviewDb;
@@ -57,13 +59,14 @@ public class IncludedGroupsCollection implements
 
   @Override
   public IncludedGroupResource parse(GroupResource resource, IdString id)
-      throws ResourceNotFoundException, OrmException {
+      throws AuthException, ResourceNotFoundException, OrmException {
     AccountGroup parent = resource.toAccountGroup();
     if (parent == null) {
       throw new ResourceNotFoundException(id);
     }
 
-    GroupDescription.Basic member = groupsCollection.get().parse(id.get());
+    GroupDescription.Basic member =
+        groupsCollection.get().parse(TopLevelResource.INSTANCE, id).getGroup();
     if (isMember(parent, member)
         && resource.getControl().canSeeGroup(member.getGroupUUID())) {
       return new IncludedGroupResource(resource, member);

gerrit-server/src/main/java/com/google/gerrit/server/group/MembersCollection.java

@@ -21,11 +21,13 @@ import com.google.gerrit.extensions.restapi.ChildCollection;
 import com.google.gerrit.extensions.restapi.IdString;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestView;
+import com.google.gerrit.extensions.restapi.TopLevelResource;
 import com.google.gerrit.reviewdb.client.AccountGroupMember;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.account.AccountsCollection;
 import com.google.gerrit.server.group.AddMembers.PutMember;
+import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 
@@ -59,12 +61,12 @@ public class MembersCollection implements
 
   @Override
   public MemberResource parse(GroupResource parent, IdString id)
-      throws ResourceNotFoundException, Exception {
+      throws AuthException, ResourceNotFoundException, OrmException {
     if (parent.toAccountGroup() == null) {
       throw new ResourceNotFoundException(id);
     }
 
-    IdentifiedUser user = accounts.get().parse(id.get());
+    IdentifiedUser user = accounts.get().parse(TopLevelResource.INSTANCE, id).getUser();
     AccountGroupMember.Key key =
         new AccountGroupMember.Key(user.getAccountId(), parent.toAccountGroup().getId());
     if (db.get().accountGroupMembers().get(key) != null

gerrit-server/src/main/java/com/google/gerrit/server/group/PutOwner.java

@@ -16,17 +16,16 @@ package com.google.gerrit.server.group;
 
 import com.google.common.base.Strings;
 import com.google.gerrit.common.data.GroupDescription;
-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.DefaultInput;
 import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
 import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.account.GroupCache;
-import com.google.gerrit.server.account.GroupControl;
 import com.google.gerrit.server.group.GroupJson.GroupInfo;
 import com.google.gerrit.server.group.PutOwner.Input;
 import com.google.gwtorm.server.OrmException;
@@ -43,24 +42,23 @@ public class PutOwner implements RestModifyView<GroupResource, Input> {
 
   private final Provider<GroupsCollection> groupsCollection;
   private final GroupCache groupCache;
-  private final GroupControl.Factory controlFactory;
   private final ReviewDb db;
   private final GroupJson json;
 
   @Inject
   PutOwner(Provider<GroupsCollection> groupsCollection, GroupCache groupCache,
-      GroupControl.Factory controlFactory, ReviewDb db, GroupJson json) {
+      ReviewDb db, GroupJson json) {
     this.groupsCollection = groupsCollection;
     this.groupCache = groupCache;
-    this.controlFactory = controlFactory;
     this.db = db;
     this.json = json;
   }
 
   @Override
   public GroupInfo apply(GroupResource resource, Input input)
-      throws MethodNotAllowedException, AuthException, BadRequestException,
+      throws ResourceNotFoundException, MethodNotAllowedException,
-      ResourceNotFoundException, OrmException {
+      AuthException, BadRequestException, UnprocessableEntityException,
+      OrmException {
     AccountGroup group = resource.toAccountGroup();
     if (group == null) {
       throw new MethodNotAllowedException();
@@ -72,28 +70,17 @@ public class PutOwner implements RestModifyView<GroupResource, Input> {
       throw new BadRequestException("owner is required");
     }
 
-    GroupDescription.Basic owner;
-    try {
-      owner = groupsCollection.get().parse(input.owner);
-    } catch (ResourceNotFoundException e) {
-      throw new BadRequestException(String.format("No such group: %s", input.owner));
-    }
-
-    try {
-      GroupControl c = controlFactory.validateFor(owner.getGroupUUID());
     group = db.accountGroups().get(group.getId());
     if (group == null) {
       throw new ResourceNotFoundException();
     }
 
+    GroupDescription.Basic owner = groupsCollection.get().parse(input.owner);
     if (!group.getOwnerGroupUUID().equals(owner.getGroupUUID())) {
       group.setOwnerGroupUUID(owner.getGroupUUID());
       db.accountGroups().update(Collections.singleton(group));
       groupCache.evict(group);
     }
-      return json.format(c.getGroup());
+    return json.format(owner);
-    } catch (NoSuchGroupException e) {
-      throw new BadRequestException(String.format("No such group: %s", input.owner));
-    }
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/project/CreateProject.java

@@ -21,7 +21,6 @@ import com.google.gerrit.common.data.GlobalCapability;
 import com.google.gerrit.common.errors.ProjectCreationFailedException;
 import com.google.gerrit.extensions.annotations.RequiresCapability;
 import com.google.gerrit.extensions.restapi.BadRequestException;
-import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.Response;
 import com.google.gerrit.extensions.restapi.RestModifyView;
 import com.google.gerrit.extensions.restapi.TopLevelResource;
@@ -91,13 +90,7 @@ class CreateProject implements RestModifyView<TopLevelResource, Input> {
     final CreateProjectArgs args = new CreateProjectArgs();
     args.setProjectName(name);
     if (!Strings.isNullOrEmpty(input.parent)) {
-      try {
+      args.newParent = projectsCollection.get().parse(input.parent).getControl();
-        args.newParent =
-            projectsCollection.get().parse(input.parent).getControl();
-      } catch (ResourceNotFoundException e) {
-        throw new UnprocessableEntityException(String.format(
-            "parent project \"%s\" not found", input.parent));
-      }
     }
     args.createEmptyCommit = input.createEmptyCommit;
     args.permissionsOnly = input.permissionsOnly;
@@ -109,12 +102,7 @@ class CreateProject implements RestModifyView<TopLevelResource, Input> {
       List<AccountGroup.UUID> ownerIds =
           Lists.newArrayListWithCapacity(input.owners.size());
       for (String owner : input.owners) {
-        try {
         ownerIds.add(groupsCollection.get().parse(owner).getGroupUUID());
-        } catch (ResourceNotFoundException e) {
-          throw new UnprocessableEntityException(String.format(
-              "group \"%s\" not found", owner));
-        }
       }
       args.ownerIds = ownerIds;
     }

gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectsCollection.java

@@ -21,6 +21,7 @@ import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestCollection;
 import com.google.gerrit.extensions.restapi.RestView;
 import com.google.gerrit.extensions.restapi.TopLevelResource;
+import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.OutputFormat;
@@ -56,21 +57,41 @@ public class ProjectsCollection implements
   @Override
   public ProjectResource parse(TopLevelResource parent, IdString id)
       throws ResourceNotFoundException {
-    return parse(id.get());
+    ProjectResource rsrc = _parse(id.get());
+    if (rsrc == null) {
+      throw new ResourceNotFoundException(id);
+    }
+    return rsrc;
   }
 
-  public ProjectResource parse(String id)
+  /**
-      throws ResourceNotFoundException {
+   * Parses a project ID from a request body and returns the project.
+   *
+   * @param id ID of the project, can be a project name
+   * @return the project
+   * @throws UnprocessableEntityException thrown if the project ID cannot be
+   *         resolved or if the project is not visible to the calling user
+   */
+  public ProjectResource parse(String id) throws UnprocessableEntityException {
+    ProjectResource rsrc = _parse(id);
+    if (rsrc == null) {
+      throw new UnprocessableEntityException(String.format(
+          "Project Not Found: %s", id));
+    }
+    return rsrc;
+  }
+
+  private ProjectResource _parse(String id) {
     ProjectControl ctl;
     try {
       ctl = controlFactory.controlFor(
           new Project.NameKey(id),
           user.get());
     } catch (NoSuchProjectException e) {
-      throw new ResourceNotFoundException(id);
+      return null;
     }
     if (!ctl.isVisible() && !ctl.isOwner()) {
-      throw new ResourceNotFoundException(id);
+      return null;
     }
     return new ProjectResource(ctl);
   }