diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/group/GroupPropertiesIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/group/GroupPropertiesIT.java index dd1afe8557..ab22367bd1 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/group/GroupPropertiesIT.java +++ b/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(); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/BadRequestHandler.java b/gerrit-server/src/main/java/com/google/gerrit/server/BadRequestHandler.java deleted file mode 100644 index 26eb85db12..0000000000 --- a/gerrit-server/src/main/java/com/google/gerrit/server/BadRequestHandler.java +++ /dev/null @@ -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 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()); - } -} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountsCollection.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountsCollection.java index c11eaec7da..4efc65eea6 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountsCollection.java +++ b/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 ", 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 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; } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java index 2bc8184836..c8f31a314c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java +++ b/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 { @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 { } } 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 { } private PostResult putGroup(ChangeResource rsrc, Input input) - throws ResourceNotFoundException, AuthException, NoSuchGroupException, - NoSuchProjectException, OrmException, NoSuchChangeException, + throws ResourceNotFoundException, AuthException, BadRequestException, + UnprocessableEntityException, OrmException, NoSuchChangeException, EmailException { - GroupDescription.Basic group = groupsCollection.get().parse(input.reviewer); - if (GroupDescriptions.toAccountGroup(group) == null) { - throw new ResourceNotFoundException(input.reviewer); - } + GroupDescription.Basic group = groupsCollection.get().parseInternal(input.reviewer); PostResult result = new PostResult(); if (!isLegalReviewerGroup(group.getGroupUUID())) { result.error = MessageFormat.format( @@ -170,8 +163,15 @@ public class PostReviewers implements RestModifyView { Set reviewers = Sets.newLinkedHashSet(); ChangeControl control = rsrc.getControl(); - Set members = groupMembersFactory.create(control.getCurrentUser()) - .listAccounts(group.getGroupUUID(), control.getProject().getNameKey()); + Set members; + 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 diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/AddIncludedGroups.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/AddIncludedGroups.java index ae9a874d8c..aa3f30e3d5 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/AddIncludedGroups.java +++ b/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 { @Override public List 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 { GroupControl control = resource.getControl(); Map newIncludedGroups = Maps.newHashMap(); List newIncludedGroupsAudits = Lists.newLinkedList(); - BadRequestHandler badRequest = new BadRequestHandler("adding groups"); List result = Lists.newLinkedList(); Account.Id me = ((IdentifiedUser) control.getCurrentUser()).getAccountId(); for (String includedGroup : input.groups) { - GroupDescription.Basic d; - try { - d = groupsCollection.get().parse(includedGroup); - } catch (ResourceNotFoundException e) { - badRequest.addError(new NoSuchGroupException(includedGroup)); - continue; - } - + GroupDescription.Basic d = groupsCollection.get().parse(includedGroup); 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 { 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 { @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 list = put.get().apply(resource, in); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/AddMembers.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/AddMembers.java index 5233e11d84..7f32a4dc56 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/AddMembers.java +++ b/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 { @Override public List apply(GroupResource resource, Input input) - throws AuthException, MethodNotAllowedException, BadRequestException, - OrmException { + throws AuthException, MethodNotAllowedException, + UnprocessableEntityException, OrmException { AccountGroup internalGroup = resource.toAccountGroup(); if (internalGroup == null) { throw new MethodNotAllowedException(); @@ -106,20 +102,14 @@ class AddMembers implements RestModifyView { GroupControl control = resource.getControl(); Map newAccountGroupMembers = Maps.newHashMap(); List newAccountGroupMemberAudits = Lists.newLinkedList(); - BadRequestHandler badRequest = new BadRequestHandler("adding new group members"); List 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())); - continue; + throw new UnprocessableEntityException(String.format( + "Account Inactive: %s", nameOrEmail)); } if (!control.canAddMember(a.getId())) { @@ -139,8 +129,6 @@ class AddMembers implements RestModifyView { 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 { 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) { - return null; - } catch (ResourceNotFoundException e) { - // might be because the account does not exist or because the account is not visible + } catch (UnprocessableEntityException e) { + // 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 { break; default: } - return null; + throw e; } } @@ -201,8 +189,8 @@ class AddMembers implements RestModifyView { @Override public Object apply(GroupResource resource, PutMember.Input input) - throws AuthException, MethodNotAllowedException, BadRequestException, - OrmException { + throws AuthException, MethodNotAllowedException, + UnprocessableEntityException, OrmException { AddMembers.Input in = new AddMembers.Input(); in._oneMember = id; List list = put.get().apply(resource, in); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/CreateGroup.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/CreateGroup.java index e2b972b709..6914cf208c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/CreateGroup.java +++ b/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 { @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 { 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.parse(Url.decode(input.ownerId)); - 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"); - } + GroupDescription.Basic d = groups.parseInternal(Url.decode(input.ownerId)); + return GroupDescriptions.toAccountGroup(d).getId(); } return null; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/DeleteIncludedGroups.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/DeleteIncludedGroups.java index fe3f669dbf..15f93258ac 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/DeleteIncludedGroups.java +++ b/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 includedGroups = getIncludedGroups(internalGroup.getId()); final List toRemove = Lists.newLinkedList(); - final BadRequestHandler badRequest = new BadRequestHandler("removing included groups"); for (final String includedGroup : input.groups) { - GroupDescription.Basic d; - try { - d = groupsCollection.get().parse(includedGroup); - } catch (ResourceNotFoundException e) { - badRequest.addError(new NoSuchGroupException(includedGroup)); - continue; - } - + GroupDescription.Basic d = groupsCollection.get().parse(includedGroup); 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 { @Override public Object apply(GroupResource resource, Input input) - throws AuthException, MethodNotAllowedException, BadRequestException, - OrmException { + throws AuthException, MethodNotAllowedException, + UnprocessableEntityException, OrmException { AccountGroup internalGroup = resource.toAccountGroup(); if (internalGroup == null) { throw new MethodNotAllowedException(); @@ -71,14 +68,9 @@ public class DeleteMembers implements RestModifyView { final GroupControl control = resource.getControl(); 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 = parse(nameOrEmail); - if (a == null) { - badRequest.addError(new NoSuchAccountException(nameOrEmail)); - continue; - } + Account a = accounts.get().parse(nameOrEmail).getAccount(); if (!control.canRemoveMember(a.getId())) { throw new AuthException("Cannot delete member: " + a.getFullName()); @@ -90,8 +82,6 @@ public class DeleteMembers implements RestModifyView { } } - badRequest.failOnError(); - writeAudits(toRemove); db.accountGroupMembers().delete(toRemove); for (final AccountGroupMember m : toRemove) { @@ -101,16 +91,6 @@ public class DeleteMembers implements RestModifyView { 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 toBeRemoved) throws OrmException { final Account.Id me = ((IdentifiedUser) self.get()).getAccountId(); @@ -161,8 +141,8 @@ public class DeleteMembers implements RestModifyView { @Override public Object apply(MemberResource resource, Input input) - throws AuthException, MethodNotAllowedException, BadRequestException, - OrmException, NoSuchGroupException { + throws AuthException, MethodNotAllowedException, + UnprocessableEntityException, OrmException, NoSuchGroupException { AddMembers.Input in = new AddMembers.Input(); in._oneMember = resource.getMember().getAccountId().toString(); return delete.get().apply(resource, in); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/GroupsCollection.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/GroupsCollection.java index 234e58f237..918a5301e0 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/GroupsCollection.java +++ b/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") diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/IncludedGroupsCollection.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/IncludedGroupsCollection.java index 789fba8481..11d12c55c1 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/IncludedGroupsCollection.java +++ b/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); 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 dd7a32cd1e..6c79bf225b 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 @@ -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 diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/PutOwner.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/PutOwner.java index 03df4d6563..ca6c06fe09 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/PutOwner.java +++ b/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 { private final Provider groupsCollection; private final GroupCache groupCache; - private final GroupControl.Factory controlFactory; private final ReviewDb db; private final GroupJson json; @Inject PutOwner(Provider 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, - ResourceNotFoundException, OrmException { + throws ResourceNotFoundException, MethodNotAllowedException, + AuthException, BadRequestException, UnprocessableEntityException, + OrmException { AccountGroup group = resource.toAccountGroup(); if (group == null) { throw new MethodNotAllowedException(); @@ -72,28 +70,17 @@ public class PutOwner implements RestModifyView { 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)); + group = db.accountGroups().get(group.getId()); + if (group == null) { + throw new ResourceNotFoundException(); } - try { - GroupControl c = controlFactory.validateFor(owner.getGroupUUID()); - group = db.accountGroups().get(group.getId()); - if (group == null) { - throw new ResourceNotFoundException(); - } - - if (!group.getOwnerGroupUUID().equals(owner.getGroupUUID())) { - group.setOwnerGroupUUID(owner.getGroupUUID()); - db.accountGroups().update(Collections.singleton(group)); - groupCache.evict(group); - } - return json.format(c.getGroup()); - } catch (NoSuchGroupException e) { - throw new BadRequestException(String.format("No such group: %s", input.owner)); + 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(owner); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateProject.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateProject.java index 51e6961e6f..5494146ab5 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateProject.java +++ b/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 { final CreateProjectArgs args = new CreateProjectArgs(); args.setProjectName(name); if (!Strings.isNullOrEmpty(input.parent)) { - try { - args.newParent = - projectsCollection.get().parse(input.parent).getControl(); - } catch (ResourceNotFoundException e) { - throw new UnprocessableEntityException(String.format( - "parent project \"%s\" not found", input.parent)); - } + args.newParent = projectsCollection.get().parse(input.parent).getControl(); } args.createEmptyCommit = input.createEmptyCommit; args.permissionsOnly = input.permissionsOnly; @@ -109,12 +102,7 @@ class CreateProject implements RestModifyView { List 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)); - } + ownerIds.add(groupsCollection.get().parse(owner).getGroupUUID()); } args.ownerIds = ownerIds; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectsCollection.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectsCollection.java index c8337265ad..4c00439ff1 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectsCollection.java +++ b/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); }