opendev/gerrit
Code Issues Proposed changes

Remove fixed limit of results returned by secondary index query

Browse Source 
The global query limit capability can be set to any value in the
project settings, however in the query implementation the limit
is hard coded to 1000.

Remove the hard-coded limit so that the limit specified in the
global capability is honoured.

Bug: Issue 2879
Change-Id: I9974f43da7cf9902b7e46607d44abc7383f77ecd
This commit is contained in:
David Pursehouse
2014-09-03 11:52:04 +09:00
parent b8bb7c1109
commit dcd2fd364e
3 changed files with 10 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
gerrit-common/src/main/java/com/google/gerrit/common/data/GlobalCapability.java
View File

@@ -70,6 +70,9 @@ public class GlobalCapability {
/** Maximum result limit per executed query. */
public static final String QUERY_LIMIT = "queryLimit";
/** Default result limit per executed query. */
public static final int DEFAULT_MAX_QUERY_LIMIT = 500;
/** Ability to impersonate another user. */
public static final String RUN_AS = "runAs";
@@ -145,7 +148,7 @@ public class GlobalCapability {
return new PermissionRange.WithDefaults(
varName,
0, Integer.MAX_VALUE,
0, 500);
0, DEFAULT_MAX_QUERY_LIMIT);
}
return null;
}

11
gerrit-server/src/main/java/com/google/gerrit/server/index/IndexRewriteImpl.java
View File

@@ -14,7 +14,8 @@
package com.google.gerrit.server.index;
import com.google.common.annotations.VisibleForTesting;
import static com.google.gerrit.common.data.GlobalCapability.DEFAULT_MAX_QUERY_LIMIT;
import com.google.common.base.Objects;
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
@@ -61,9 +62,6 @@ public class IndexRewriteImpl implements ChangeQueryRewriter {
CLOSED_STATUSES = Sets.immutableEnumSet(closed);
}
@VisibleForTesting
static final int MAX_LIMIT = 1000;
/**
* Get the set of statuses that changes matching the given predicate may have.
*
@@ -135,13 +133,12 @@ public class IndexRewriteImpl implements ChangeQueryRewriter {
throws QueryParseException {
ChangeIndex index = indexes.getSearchIndex();
in = basicRewrites.rewrite(in);
int limit =
Objects.firstNonNull(ChangeQueryBuilder.getLimit(in), MAX_LIMIT);
int limit = Objects.firstNonNull(
ChangeQueryBuilder.getLimit(in), DEFAULT_MAX_QUERY_LIMIT);
// Increase the limit rather than skipping, since we don't know how many
// skipped results would have been filtered out by the enclosing AndSource.
limit += start;
limit = Math.max(limit, 1);
limit = Math.min(limit, MAX_LIMIT);
Predicate<ChangeData> out = rewriteImpl(in, index, limit);
if (in == out || out instanceof IndexPredicate) {

10
gerrit-server/src/test/java/com/google/gerrit/server/index/IndexRewriteTest.java
View File

@@ -14,6 +14,7 @@
package com.google.gerrit.server.index;
import static com.google.gerrit.common.data.GlobalCapability.DEFAULT_MAX_QUERY_LIMIT;
import static com.google.gerrit.reviewdb.client.Change.Status.ABANDONED;
import static com.google.gerrit.reviewdb.client.Change.Status.DRAFT;
import static com.google.gerrit.reviewdb.client.Change.Status.MERGED;
@@ -179,13 +180,6 @@ public class IndexRewriteTest {
assertEquals(and(query(f, 5), l), rewrite.rewrite(in, 2));
}
@Test
public void testStartDoesNotExceedMaxLimit() throws Exception {
Predicate<ChangeData> in = parse("file:a");
assertEquals(query(in), rewrite.rewrite(in, 0));
assertEquals(query(in), rewrite.rewrite(in, 1));
}
@Test
public void testGetPossibleStatus() throws Exception {
assertEquals(EnumSet.allOf(Change.Status.class), status("file:a"));
@@ -233,7 +227,7 @@ public class IndexRewriteTest {
private IndexedChangeQuery query(Predicate<ChangeData> p)
throws QueryParseException {
return query(p, IndexRewriteImpl.MAX_LIMIT);
return query(p, DEFAULT_MAX_QUERY_LIMIT);
}
private IndexedChangeQuery query(Predicate<ChangeData> p, int limit)