ListGroups: Remove deprecated --query option

And rename --query2 to --query on QueryGroups.

Remove rest/group/GroupsIT which only contains a single test of
sending an invalid combination of --query and --query2.

Change-Id: I1a1bdbbcd49b04f882ee3e309a2e3bbc34ccd6ba

Documentation/rest-api-groups.txt

@@ -341,24 +341,19 @@ List all groups that match substring `test/`:
 [[query-groups]]
 === Query Groups
 --
-'GET /groups/?query2=<query>'
+'GET /groups/?query=<query>'
 --
 
 Queries internal groups visible to the caller. The
 link:user-search-groups.html#_search_operators[query string] must be
-provided by the `query2` parameter. The `start` and `limit` parameters
+provided by the `query` parameter. The `start` and `limit` parameters
 can be used to skip/limit results.
 
 As result a list of link:#group-info[GroupInfo] entities is returned.
 
-[NOTE] `query2` is a temporary name and in future this option may be
-renamed to `query`. `query2` was chosen to maintain backwards
-compatibility with the deprecated `query` parameter on the
-link:#list-groups[List Groups] endpoint.
-
 .Request
 ----
-  GET /groups/?query2=inname:test HTTP/1.0
+  GET /groups/?query=inname:test HTTP/1.0
 ----
 
 .Response
@@ -398,12 +393,12 @@ a `_more_groups: true` JSON field set.
 
 [[group-query-limit]]
 ==== Group Limit
-The `/groups/?query2=<query>` URL also accepts a limit integer in the
+The `/groups/?query=<query>` URL also accepts a limit integer in the
 `limit` parameter. This limits the results to `limit` groups.
 
 Query the first 25 groups in group list.
 ----
-  GET /groups/?query2=<query>&limit=25 HTTP/1.0
+  GET /groups/?query=<query>&limit=25 HTTP/1.0
 ----
 
 The `/groups/` URL also accepts a start integer in the `start`
@@ -411,7 +406,7 @@ parameter. The results will skip `start` groups from group list.
 
 Query 25 groups starting from index 50.
 ----
-  GET /groups/?query2=<query>&limit=25&start=50 HTTP/1.0
+  GET /groups/?query=<query>&limit=25&start=50 HTTP/1.0
 ----
 
 [[group-query-options]]

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

@@ -14,13 +14,10 @@
 
 package com.google.gerrit.server.restapi.group;
 
-import com.google.common.collect.ListMultimap;
 import com.google.gerrit.common.data.GroupDescription;
 import com.google.gerrit.extensions.registration.DynamicMap;
 import com.google.gerrit.extensions.restapi.AuthException;
-import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.IdString;
-import com.google.gerrit.extensions.restapi.NeedsParams;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestCollection;
 import com.google.gerrit.extensions.restapi.RestView;
@@ -33,43 +30,27 @@ import com.google.gerrit.server.group.GroupResource;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 
-public class GroupsCollection
+public class GroupsCollection implements RestCollection<TopLevelResource, GroupResource> {
-    implements RestCollection<TopLevelResource, GroupResource>, NeedsParams {
   private final DynamicMap<RestView<GroupResource>> views;
-  private final Provider<ListGroups> list;
   private final Provider<QueryGroups> queryGroups;
   private final GroupControl.Factory groupControlFactory;
   private final GroupResolver groupResolver;
   private final Provider<CurrentUser> self;
 
-  private boolean hasQuery2;
-
   @Inject
   public GroupsCollection(
       DynamicMap<RestView<GroupResource>> views,
-      Provider<ListGroups> list,
       Provider<QueryGroups> queryGroups,
       GroupControl.Factory groupControlFactory,
       GroupResolver groupResolver,
       Provider<CurrentUser> self) {
     this.views = views;
-    this.list = list;
     this.queryGroups = queryGroups;
     this.groupControlFactory = groupControlFactory;
     this.groupResolver = groupResolver;
     this.self = self;
   }
 
-  @Override
-  public void setParams(ListMultimap<String, String> params) throws BadRequestException {
-    if (params.containsKey("query") && params.containsKey("query2")) {
-      throw new BadRequestException("\"query\" and \"query2\" options are mutually exclusive");
-    }
-
-    // The --query2 option is defined in QueryGroups
-    this.hasQuery2 = params.containsKey("query2");
-  }
-
   @Override
   public RestView<TopLevelResource> list() throws ResourceNotFoundException, AuthException {
     final CurrentUser user = self.get();
@@ -78,12 +59,7 @@ public class GroupsCollection
     } else if (!(user.isIdentifiedUser())) {
       throw new ResourceNotFoundException();
     }
-
+    return queryGroups.get();
-    if (hasQuery2) {
-      return queryGroups.get();
-    }
-
-    return list.get();
   }
 
   @Override

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

@@ -129,21 +129,6 @@ public class ListGroups implements RestReadView<TopLevelResource> {
     this.owned = owned;
   }
 
-  /**
-   * Add a group to inspect.
-   *
-   * @param uuid UUID of the group
-   * @deprecated use {@link #addGroup(AccountGroup.UUID)}.
-   */
-  @Deprecated
-  @Option(
-      name = "--query",
-      aliases = {"-q"},
-      usage = "group to inspect (deprecated: use --group/-g instead)")
-  void addGroup_Deprecated(AccountGroup.UUID uuid) {
-    addGroup(uuid);
-  }
-
   @Option(
       name = "--group",
       aliases = {"-g"},

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

@@ -48,12 +48,9 @@ public class QueryGroups implements RestReadView<TopLevelResource> {
   private int start;
   private EnumSet<ListGroupsOption> options = EnumSet.noneOf(ListGroupsOption.class);
 
-  // TODO(ekempin): --query in ListGroups is marked as deprecated, once it is
-  // removed we want to rename --query2 to --query here.
-  /** --query (-q) is already used by {@link ListGroups} */
   @Option(
-      name = "--query2",
+      name = "--query",
-      aliases = {"-q2"},
+      aliases = {"-q"},
       usage = "group query")
   public void setQuery(String query) {
     this.query = query;

javatests/com/google/gerrit/acceptance/rest/group/GroupsIT.java

@@ -1,31 +0,0 @@
-// Copyright (C) 2017 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.acceptance.rest.group;
-
-import static com.google.common.truth.Truth.assertThat;
-
-import com.google.gerrit.acceptance.AbstractDaemonTest;
-import com.google.gerrit.acceptance.RestResponse;
-import org.junit.Test;
-
-public class GroupsIT extends AbstractDaemonTest {
-  @Test
-  public void invalidQueryOptions() throws Exception {
-    RestResponse r = adminRestSession.put("/groups/?query=foo&query2=bar");
-    r.assertBadRequest();
-    assertThat(r.getEntityContent())
-        .isEqualTo("\"query\" and \"query2\" options are mutually exclusive");
-  }
-}