Improve and document REST API errors on non-internal groups

Many REST API endpoints require an internal group and return 405 if the
group is not an internal group. This is a non-obvious requirement and
contract of API, so make it explicit in the documentation.

Provide a more descriptive message in the error text, and add a new
subclass NotInternalGroupException to avoid the need to repeat ourselves
in each endpoint handler.

Change-Id: I993ebf98ce4eb565b9f98ff10742114d57ce6f9a

Documentation/rest-api-groups.txt

@@ -597,6 +597,9 @@ Renames a Gerrit internal group.
 
 The new group name must be provided in the request body.
 
+This endpoint is only allowed for Gerrit internal groups; attempting to call on a
+non-internal group will return 405 Method Not Allowed.
+
 .Request
 ----
   PUT /groups/MyProject-Committers/name HTTP/1.0
@@ -630,6 +633,9 @@ response is "`409 Conflict`".
 
 Retrieves the description of a group.
 
+This endpoint is only allowed for Gerrit internal groups; attempting to call on a
+non-internal group will return 405 Method Not Allowed.
+
 .Request
 ----
   GET /groups/9999c971bb4ab872aab759d8c49833ee6b9ff320/description HTTP/1.0
@@ -657,6 +663,9 @@ Sets the description of a Gerrit internal group.
 
 The new group description must be provided in the request body.
 
+This endpoint is only allowed for Gerrit internal groups; attempting to call on a
+non-internal group will return 405 Method Not Allowed.
+
 .Request
 ----
   PUT /groups/9999c971bb4ab872aab759d8c49833ee6b9ff320/description HTTP/1.0
@@ -738,6 +747,9 @@ Sets the options of a Gerrit internal group.
 The new group options must be provided in the request body as a
 link:#group-options-input[GroupOptionsInput] entity.
 
+This endpoint is only allowed for Gerrit internal groups; attempting to call on a
+non-internal group will return 405 Method Not Allowed.
+
 .Request
 ----
   PUT /groups/9999c971bb4ab872aab759d8c49833ee6b9ff320/options HTTP/1.0
@@ -771,6 +783,9 @@ link:#group-options-info[GroupOptionsInfo] entity.
 
 Retrieves the owner group of a Gerrit internal group.
 
+This endpoint is only allowed for Gerrit internal groups; attempting to call on a
+non-internal group will return 405 Method Not Allowed.
+
 .Request
 ----
   GET /groups/9999c971bb4ab872aab759d8c49833ee6b9ff320/owner HTTP/1.0
@@ -813,6 +828,9 @@ The new owner group must be provided in the request body.
 The new owner can be specified by name, by group UUID or by the legacy
 numeric group ID.
 
+This endpoint is only allowed for Gerrit internal groups; attempting to call on a
+non-internal group will return 405 Method Not Allowed.
+
 .Request
 ----
   PUT /groups/9999c971bb4ab872aab759d8c49833ee6b9ff320/owner HTTP/1.0
@@ -855,6 +873,9 @@ describes the new owner group.
 
 Gets the audit log of a Gerrit internal group.
 
+This endpoint is only allowed for Gerrit internal groups; attempting to call on a
+non-internal group will return 405 Method Not Allowed.
+
 .Request
 ----
   GET /groups/9999c971bb4ab872aab759d8c49833ee6b9ff320/log.audit HTTP/1.0
@@ -965,6 +986,9 @@ As result a list of detailed link:rest-api-accounts.html#account-info[
 AccountInfo] entries is returned. The entries in the list are sorted by
 full name, preferred email and id.
 
+This endpoint is only allowed for Gerrit internal groups; attempting to call on a
+non-internal group will return 405 Method Not Allowed.
+
 .Request
 ----
   GET /groups/834ec36dd5e0ed21a2ff5d7e2255da082d63bbd7/members/ HTTP/1.0
@@ -1077,6 +1101,9 @@ AccountInfo] entity is returned that describes the group member.
 
 Adds a user as member to a Gerrit internal group.
 
+This endpoint is only allowed for Gerrit internal groups; attempting to call on a
+non-internal group will return 405 Method Not Allowed.
+
 .Request
 ----
   PUT /groups/MyProject-Committers/members/John%20Doe HTTP/1.0
@@ -1171,6 +1198,9 @@ already a member of the group.
 
 Removes a user from a Gerrit internal group.
 
+This endpoint is only allowed for Gerrit internal groups; attempting to call on a
+non-internal group will return 405 Method Not Allowed.
+
 .Request
 ----
   DELETE /groups/MyProject-Committers/members/John%20Doe HTTP/1.0
@@ -1224,6 +1254,9 @@ Lists the direct subgroups of a group.
 As result a list of link:#group-info[GroupInfo] entries is returned.
 The entries in the list are sorted by group name and UUID.
 
+This endpoint is only allowed for Gerrit internal groups; attempting to call on a
+non-internal group will return 405 Method Not Allowed.
+
 .Request
 ----
   GET /groups/834ec36dd5e0ed21a2ff5d7e2255da082d63bbd7/groups/ HTTP/1.0
@@ -1296,6 +1329,9 @@ describes the subgroup.
 Adds an internal or external group as subgroup to a Gerrit internal group.
 External groups must be specified using the UUID.
 
+This endpoint is only allowed for Gerrit internal groups; attempting to call on a
+non-internal group will return 405 Method Not Allowed.
+
 .Request
 ----
   PUT /groups/MyProject-Committers/groups/MyGroup HTTP/1.0
@@ -1405,6 +1441,9 @@ group was already a subgroup of the group.
 
 Removes a subgroup from a Gerrit internal group.
 
+This endpoint is only allowed for Gerrit internal groups; attempting to call on a
+non-internal group will return 405 Method Not Allowed.
+
 .Request
 ----
   DELETE /groups/MyProject-Committers/groups/MyGroup HTTP/1.0

java/com/google/gerrit/server/restapi/group/AddMembers.java

@@ -117,10 +117,10 @@ public class AddMembers implements RestModifyView<GroupResource, Input> {
 
   @Override
   public List<AccountInfo> apply(GroupResource resource, Input input)
-      throws AuthException, MethodNotAllowedException, UnprocessableEntityException, OrmException,
+      throws AuthException, NotInternalGroupException, UnprocessableEntityException, OrmException,
           IOException, ConfigInvalidException, ResourceNotFoundException {
     GroupDescription.Internal internalGroup =
-        resource.asInternalGroup().orElseThrow(MethodNotAllowedException::new);
+        resource.asInternalGroup().orElseThrow(NotInternalGroupException::new);
     input = Input.init(input);
 
     GroupControl control = resource.getControl();

java/com/google/gerrit/server/restapi/group/AddSubgroups.java

@@ -93,10 +93,10 @@ public class AddSubgroups implements RestModifyView<GroupResource, Input> {
 
   @Override
   public List<GroupInfo> apply(GroupResource resource, Input input)
-      throws MethodNotAllowedException, AuthException, UnprocessableEntityException, OrmException,
+      throws NotInternalGroupException, AuthException, UnprocessableEntityException, OrmException,
           ResourceNotFoundException, IOException, ConfigInvalidException {
     GroupDescription.Internal group =
-        resource.asInternalGroup().orElseThrow(MethodNotAllowedException::new);
+        resource.asInternalGroup().orElseThrow(NotInternalGroupException::new);
     input = Input.init(input);
 
     GroupControl control = resource.getControl();

java/com/google/gerrit/server/restapi/group/DeleteMembers.java

@@ -61,10 +61,10 @@ public class DeleteMembers implements RestModifyView<GroupResource, Input> {
 
   @Override
   public Response<?> apply(GroupResource resource, Input input)
-      throws AuthException, MethodNotAllowedException, UnprocessableEntityException, OrmException,
+      throws AuthException, NotInternalGroupException, UnprocessableEntityException, OrmException,
           IOException, ConfigInvalidException, ResourceNotFoundException {
     GroupDescription.Internal internalGroup =
-        resource.asInternalGroup().orElseThrow(MethodNotAllowedException::new);
+        resource.asInternalGroup().orElseThrow(NotInternalGroupException::new);
     input = Input.init(input);
 
     final GroupControl control = resource.getControl();

java/com/google/gerrit/server/restapi/group/DeleteSubgroups.java

@@ -60,10 +60,10 @@ public class DeleteSubgroups implements RestModifyView<GroupResource, Input> {
 
   @Override
   public Response<?> apply(GroupResource resource, Input input)
-      throws AuthException, MethodNotAllowedException, UnprocessableEntityException, OrmException,
+      throws AuthException, NotInternalGroupException, UnprocessableEntityException, OrmException,
           ResourceNotFoundException, IOException, ConfigInvalidException {
     GroupDescription.Internal internalGroup =
-        resource.asInternalGroup().orElseThrow(MethodNotAllowedException::new);
+        resource.asInternalGroup().orElseThrow(NotInternalGroupException::new);
     input = Input.init(input);
 
     final GroupControl control = resource.getControl();

java/com/google/gerrit/server/restapi/group/GetAuditLog.java

@@ -21,7 +21,6 @@ import com.google.gerrit.extensions.common.AccountInfo;
 import com.google.gerrit.extensions.common.GroupAuditEventInfo;
 import com.google.gerrit.extensions.common.GroupInfo;
 import com.google.gerrit.extensions.restapi.AuthException;
-import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
 import com.google.gerrit.extensions.restapi.RestReadView;
 import com.google.gerrit.extensions.restapi.Url;
 import com.google.gerrit.reviewdb.client.AccountGroup;
@@ -82,10 +81,10 @@ public class GetAuditLog implements RestReadView<GroupResource> {
 
   @Override
   public List<? extends GroupAuditEventInfo> apply(GroupResource rsrc)
-      throws AuthException, MethodNotAllowedException, OrmException, IOException,
+      throws AuthException, NotInternalGroupException, OrmException, IOException,
           ConfigInvalidException {
     GroupDescription.Internal group =
-        rsrc.asInternalGroup().orElseThrow(MethodNotAllowedException::new);
+        rsrc.asInternalGroup().orElseThrow(NotInternalGroupException::new);
     if (!rsrc.getControl().isOwner()) {
       throw new AuthException("Not group owner");
     }

java/com/google/gerrit/server/restapi/group/GetDescription.java

@@ -16,7 +16,6 @@ package com.google.gerrit.server.restapi.group;
 
 import com.google.common.base.Strings;
 import com.google.gerrit.common.data.GroupDescription;
-import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
 import com.google.gerrit.extensions.restapi.RestReadView;
 import com.google.gerrit.server.group.GroupResource;
 import com.google.inject.Singleton;
@@ -24,9 +23,9 @@ import com.google.inject.Singleton;
 @Singleton
 public class GetDescription implements RestReadView<GroupResource> {
   @Override
-  public String apply(GroupResource resource) throws MethodNotAllowedException {
+  public String apply(GroupResource resource) throws NotInternalGroupException {
     GroupDescription.Internal group =
-        resource.asInternalGroup().orElseThrow(MethodNotAllowedException::new);
+        resource.asInternalGroup().orElseThrow(NotInternalGroupException::new);
     return Strings.nullToEmpty(group.getDescription());
   }
 }

java/com/google/gerrit/server/restapi/group/GetOwner.java

@@ -17,7 +17,6 @@ package com.google.gerrit.server.restapi.group;
 import com.google.gerrit.common.data.GroupDescription;
 import com.google.gerrit.common.errors.NoSuchGroupException;
 import com.google.gerrit.extensions.common.GroupInfo;
-import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestReadView;
 import com.google.gerrit.server.account.GroupControl;
@@ -40,9 +39,9 @@ public class GetOwner implements RestReadView<GroupResource> {
 
   @Override
   public GroupInfo apply(GroupResource resource)
-      throws MethodNotAllowedException, ResourceNotFoundException, OrmException {
+      throws NotInternalGroupException, ResourceNotFoundException, OrmException {
     GroupDescription.Internal group =
-        resource.asInternalGroup().orElseThrow(MethodNotAllowedException::new);
+        resource.asInternalGroup().orElseThrow(NotInternalGroupException::new);
     try {
       GroupControl c = controlFactory.validateFor(group.getOwnerGroupUUID());
       return json.format(c.getGroup());

java/com/google/gerrit/server/restapi/group/ListMembers.java

@@ -22,7 +22,6 @@ import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
 import com.google.gerrit.common.data.GroupDescription;
 import com.google.gerrit.extensions.common.AccountInfo;
-import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
 import com.google.gerrit.extensions.restapi.RestReadView;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountGroup;
@@ -67,9 +66,9 @@ public class ListMembers implements RestReadView<GroupResource> {
 
   @Override
   public List<AccountInfo> apply(GroupResource resource)
-      throws MethodNotAllowedException, OrmException {
+      throws NotInternalGroupException, OrmException {
     GroupDescription.Internal group =
-        resource.asInternalGroup().orElseThrow(MethodNotAllowedException::new);
+        resource.asInternalGroup().orElseThrow(NotInternalGroupException::new);
     if (recursive) {
       return getTransitiveMembers(group, resource.getControl());
     }

java/com/google/gerrit/server/restapi/group/ListSubgroups.java

@@ -19,7 +19,6 @@ import static com.google.common.base.Strings.nullToEmpty;
 import com.google.gerrit.common.data.GroupDescription;
 import com.google.gerrit.common.errors.NoSuchGroupException;
 import com.google.gerrit.extensions.common.GroupInfo;
-import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
 import com.google.gerrit.extensions.restapi.RestReadView;
 import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.server.account.GroupControl;
@@ -47,9 +46,9 @@ public class ListSubgroups implements RestReadView<GroupResource> {
   }
 
   @Override
-  public List<GroupInfo> apply(GroupResource rsrc) throws MethodNotAllowedException, OrmException {
+  public List<GroupInfo> apply(GroupResource rsrc) throws NotInternalGroupException, OrmException {
     GroupDescription.Internal group =
-        rsrc.asInternalGroup().orElseThrow(MethodNotAllowedException::new);
+        rsrc.asInternalGroup().orElseThrow(NotInternalGroupException::new);
 
     return getDirectSubgroups(group, rsrc.getControl());
   }

java/com/google/gerrit/server/restapi/group/MembersCollection.java

@@ -20,7 +20,6 @@ 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.MethodNotAllowedException;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestView;
 import com.google.gerrit.extensions.restapi.TopLevelResource;
@@ -63,10 +62,10 @@ public class MembersCollection
 
   @Override
   public MemberResource parse(GroupResource parent, IdString id)
-      throws MethodNotAllowedException, AuthException, ResourceNotFoundException, OrmException,
+      throws NotInternalGroupException, AuthException, ResourceNotFoundException, OrmException,
           IOException, ConfigInvalidException {
     GroupDescription.Internal group =
-        parent.asInternalGroup().orElseThrow(MethodNotAllowedException::new);
+        parent.asInternalGroup().orElseThrow(NotInternalGroupException::new);
 
     IdentifiedUser user = accounts.parse(TopLevelResource.INSTANCE, id).getUser();
     if (parent.getControl().canSeeMember(user.getAccountId()) && isMember(group, user)) {

java/com/google/gerrit/server/restapi/group/NotInternalGroupException.java

@@ -0,0 +1,25 @@
+// Copyright (C) 2018 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.restapi.group;
+
+import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
+
+class NotInternalGroupException extends MethodNotAllowedException {
+  private static final long serialVersionUID = 1L;
+
+  NotInternalGroupException() {
+    super("not a Gerrit internal group");
+  }
+}

java/com/google/gerrit/server/restapi/group/PutDescription.java

@@ -19,7 +19,6 @@ import com.google.gerrit.common.data.GroupDescription;
 import com.google.gerrit.common.errors.NoSuchGroupException;
 import com.google.gerrit.extensions.common.DescriptionInput;
 import com.google.gerrit.extensions.restapi.AuthException;
-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;
@@ -51,14 +50,14 @@ public class PutDescription implements RestModifyView<GroupResource, Description
 
   @Override
   public Response<String> apply(GroupResource resource, DescriptionInput input)
-      throws AuthException, MethodNotAllowedException, ResourceNotFoundException, OrmException,
+      throws AuthException, NotInternalGroupException, ResourceNotFoundException, OrmException,
           IOException, ConfigInvalidException {
     if (input == null) {
       input = new DescriptionInput(); // Delete would set description to null.
     }
 
     GroupDescription.Internal internalGroup =
-        resource.asInternalGroup().orElseThrow(MethodNotAllowedException::new);
+        resource.asInternalGroup().orElseThrow(NotInternalGroupException::new);
     if (!resource.getControl().isOwner()) {
       throw new AuthException("Not group owner");
     }

java/com/google/gerrit/server/restapi/group/PutName.java

@@ -20,7 +20,6 @@ import com.google.gerrit.common.errors.NoSuchGroupException;
 import com.google.gerrit.extensions.common.NameInput;
 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.ResourceConflictException;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestModifyView;
@@ -51,11 +50,11 @@ public class PutName implements RestModifyView<GroupResource, NameInput> {
 
   @Override
   public String apply(GroupResource rsrc, NameInput input)
-      throws MethodNotAllowedException, AuthException, BadRequestException,
+      throws NotInternalGroupException, AuthException, BadRequestException,
           ResourceConflictException, ResourceNotFoundException, OrmException, IOException,
           ConfigInvalidException {
     GroupDescription.Internal internalGroup =
-        rsrc.asInternalGroup().orElseThrow(MethodNotAllowedException::new);
+        rsrc.asInternalGroup().orElseThrow(NotInternalGroupException::new);
     if (!rsrc.getControl().isOwner()) {
       throw new AuthException("Not group owner");
     } else if (input == null || Strings.isNullOrEmpty(input.name)) {

java/com/google/gerrit/server/restapi/group/PutOptions.java

@@ -19,7 +19,6 @@ import com.google.gerrit.common.errors.NoSuchGroupException;
 import com.google.gerrit.extensions.common.GroupOptionsInfo;
 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.RestModifyView;
 import com.google.gerrit.reviewdb.client.AccountGroup;
@@ -48,10 +47,10 @@ public class PutOptions implements RestModifyView<GroupResource, GroupOptionsInf
 
   @Override
   public GroupOptionsInfo apply(GroupResource resource, GroupOptionsInfo input)
-      throws MethodNotAllowedException, AuthException, BadRequestException,
+      throws NotInternalGroupException, AuthException, BadRequestException,
           ResourceNotFoundException, OrmException, IOException, ConfigInvalidException {
     GroupDescription.Internal internalGroup =
-        resource.asInternalGroup().orElseThrow(MethodNotAllowedException::new);
+        resource.asInternalGroup().orElseThrow(NotInternalGroupException::new);
     if (!resource.getControl().isOwner()) {
       throw new AuthException("Not group owner");
     }

java/com/google/gerrit/server/restapi/group/PutOwner.java

@@ -21,7 +21,6 @@ import com.google.gerrit.extensions.api.groups.OwnerInput;
 import com.google.gerrit.extensions.common.GroupInfo;
 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.RestModifyView;
 import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
@@ -59,11 +58,11 @@ public class PutOwner implements RestModifyView<GroupResource, OwnerInput> {
 
   @Override
   public GroupInfo apply(GroupResource resource, OwnerInput input)
-      throws ResourceNotFoundException, MethodNotAllowedException, AuthException,
+      throws ResourceNotFoundException, NotInternalGroupException, AuthException,
           BadRequestException, UnprocessableEntityException, OrmException, IOException,
           ConfigInvalidException {
     GroupDescription.Internal internalGroup =
-        resource.asInternalGroup().orElseThrow(MethodNotAllowedException::new);
+        resource.asInternalGroup().orElseThrow(NotInternalGroupException::new);
     if (!resource.getControl().isOwner()) {
       throw new AuthException("Not group owner");
     }

java/com/google/gerrit/server/restapi/group/SubgroupsCollection.java

@@ -20,7 +20,6 @@ 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.MethodNotAllowedException;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestView;
 import com.google.gerrit.extensions.restapi.TopLevelResource;
@@ -57,9 +56,9 @@ public class SubgroupsCollection
 
   @Override
   public SubgroupResource parse(GroupResource resource, IdString id)
-      throws MethodNotAllowedException, AuthException, ResourceNotFoundException {
+      throws NotInternalGroupException, AuthException, ResourceNotFoundException {
     GroupDescription.Internal parent =
-        resource.asInternalGroup().orElseThrow(MethodNotAllowedException::new);
+        resource.asInternalGroup().orElseThrow(NotInternalGroupException::new);
 
     GroupDescription.Basic member =
         groupsCollection.parse(TopLevelResource.INSTANCE, id).getGroup();