Merge "ChangeUpdate#setStatus: Fix error message"
diff --git a/Documentation/rest-api-groups.txt b/Documentation/rest-api-groups.txt
index 00fd81f..a99c3bb 100644
--- a/Documentation/rest-api-groups.txt
+++ b/Documentation/rest-api-groups.txt
@@ -341,24 +341,19 @@
[[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 @@
[[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 @@
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]]
diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index 7edb43a..fd61aa5 100644
--- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -644,6 +644,14 @@
return result;
}
+ protected PushOneCommit.Result createChange(TestRepository<InMemoryRepository> repo)
+ throws Exception {
+ PushOneCommit push = pushFactory.create(admin.newIdent(), repo);
+ PushOneCommit.Result result = push.to("refs/for/master");
+ result.assertOkStatus();
+ return result;
+ }
+
protected PushOneCommit.Result createMergeCommitChange(String ref) throws Exception {
return createMergeCommitChange(ref, "foo");
}
diff --git a/java/com/google/gerrit/server/restapi/group/GroupsCollection.java b/java/com/google/gerrit/server/restapi/group/GroupsCollection.java
index 52fe9d0..b92a464 100644
--- a/java/com/google/gerrit/server/restapi/group/GroupsCollection.java
+++ b/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,27 +30,21 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
-public class GroupsCollection
- implements RestCollection<TopLevelResource, GroupResource>, NeedsParams {
+public class GroupsCollection implements RestCollection<TopLevelResource, GroupResource> {
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;
@@ -61,16 +52,6 @@
}
@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();
if (user instanceof AnonymousUser) {
@@ -78,12 +59,7 @@
} else if (!(user.isIdentifiedUser())) {
throw new ResourceNotFoundException();
}
-
- if (hasQuery2) {
- return queryGroups.get();
- }
-
- return list.get();
+ return queryGroups.get();
}
@Override
diff --git a/java/com/google/gerrit/server/restapi/group/ListGroups.java b/java/com/google/gerrit/server/restapi/group/ListGroups.java
index 899ed00..adc251c 100644
--- a/java/com/google/gerrit/server/restapi/group/ListGroups.java
+++ b/java/com/google/gerrit/server/restapi/group/ListGroups.java
@@ -129,21 +129,6 @@
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"},
diff --git a/java/com/google/gerrit/server/restapi/group/QueryGroups.java b/java/com/google/gerrit/server/restapi/group/QueryGroups.java
index a233111..380d42e 100644
--- a/java/com/google/gerrit/server/restapi/group/QueryGroups.java
+++ b/java/com/google/gerrit/server/restapi/group/QueryGroups.java
@@ -48,12 +48,9 @@
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",
- aliases = {"-q2"},
+ name = "--query",
+ aliases = {"-q"},
usage = "group query")
public void setQuery(String query) {
this.query = query;
diff --git a/javatests/com/google/gerrit/acceptance/api/change/QueryChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/QueryChangeIT.java
index 2cae04a..54c5d05 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/QueryChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/QueryChangeIT.java
@@ -15,22 +15,31 @@
package com.google.gerrit.acceptance.api.change;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.block;
+import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
+import static java.util.stream.Collectors.toList;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.acceptance.UseClockStep;
import com.google.gerrit.acceptance.config.GerritConfig;
+import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
+import com.google.gerrit.common.data.Permission;
+import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.restapi.TopLevelResource;
import com.google.gerrit.server.restapi.change.QueryChanges;
import com.google.inject.Inject;
import com.google.inject.Provider;
import java.util.List;
+import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
+import org.eclipse.jgit.junit.TestRepository;
import org.junit.Test;
@NoHttpd
public class QueryChangeIT extends AbstractDaemonTest {
-
+ @Inject private ProjectOperations projectOperations;
@Inject private Provider<QueryChanges> queryChangesProvider;
@Test
@@ -123,6 +132,56 @@
assertThat(result.get(2).get(0)._number).isEqualTo(numericId2);
}
+ @Test
+ @UseClockStep
+ @SuppressWarnings("unchecked")
+ public void withPagedResults() throws Exception {
+ // Create 4 visible changes.
+ createChange(testRepo).getChange().getId().get();
+ createChange(testRepo).getChange().getId().get();
+ int changeId3 = createChange(testRepo).getChange().getId().get();
+ int changeId4 = createChange(testRepo).getChange().getId().get();
+
+ // Create hidden project.
+ Project.NameKey hiddenProject = projectOperations.newProject().create();
+ projectOperations
+ .project(hiddenProject)
+ .forUpdate()
+ .add(block(Permission.READ).ref("refs/*").group(REGISTERED_USERS))
+ .update();
+ TestRepository<InMemoryRepository> hiddenRepo = cloneProject(hiddenProject, admin);
+
+ // Create 2 hidden changes.
+ createChange(hiddenRepo);
+ createChange(hiddenRepo);
+
+ // Create a change query that matches all changes (visible and hidden changes).
+ // The index returns the changes ordered by last updated timestamp:
+ // hiddenChange2, hiddenChange1, change4, change3, change2, change1
+ QueryChanges queryChanges = queryChangesProvider.get();
+ queryChanges.addQuery("branch:master");
+
+ // Set a limit on the query so that we need to paginate over the results from the index.
+ queryChanges.setLimit(2);
+
+ // Execute the query and verify the results.
+ // Since the limit is set to 2, at most 2 changes are returned to user, but the index query is
+ // executed with limit 3 (+1 so that we can populate the _more_changes field on the last
+ // result).
+ // This means the index query with limit 3 returns these changes:
+ // hiddenChange2, hiddenChange1, change4
+ // The 2 hidden changes are filtered out because they are not visible to the caller.
+ // This means we have only one matching result (change4) but the limit (3) is not exhausted
+ // yet. Hence the next page is loaded from the index (startIndex is 3 to skip the results
+ // that we already processed, limit is again 3). The results for the next page are:
+ // change3, change2, change1
+ // change2 and change1 are dropped because they are over the limit.
+ List<ChangeInfo> result =
+ (List<ChangeInfo>) queryChanges.apply(TopLevelResource.INSTANCE).value();
+ assertThat(result.stream().map(i -> i._number).collect(toList()))
+ .containsExactly(changeId3, changeId4);
+ }
+
private static void assertNoChangeHasMoreChangesSet(List<ChangeInfo> results) {
for (ChangeInfo info : results) {
assertThat(info._moreChanges).isNull();
diff --git a/javatests/com/google/gerrit/acceptance/rest/group/GroupsIT.java b/javatests/com/google/gerrit/acceptance/rest/group/GroupsIT.java
deleted file mode 100644
index e153e561..0000000
--- a/javatests/com/google/gerrit/acceptance/rest/group/GroupsIT.java
+++ /dev/null
@@ -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");
- }
-}
