Merge changes from topic 'max-pages'
* changes:
IndexConfig: Add maxPages option
IndexConfig: Configure maxLimit from Config
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 61c764a..8119d4a 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -2205,6 +2205,24 @@
+
Defaults to true.
+[[index.maxLimit]]index.maxLimit::
++
+Maximum limit to allow for search queries. Requesting results above this
+limit will truncate the list (but will still set `_more_changes` on
+result lists). Set to 0 for no limit.
++
+Defaults to no limit.
+
+[[index.maxPages]]index.maxPages::
++
+Maximum number of pages of search results to allow, as index
+implementations may have to scan through large numbers of skipped
+results when searching with an offset. Requesting results starting past
+this threshold times the requested limit will result in an error. Set to
+0 for no limit.
++
+Defaults to no limit.
+
==== Lucene configuration
Open and closed changes are indexed in separate indexes named
diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneIndexModule.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneIndexModule.java
index 0fc11d9..35d6636 100644
--- a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneIndexModule.java
+++ b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneIndexModule.java
@@ -16,6 +16,7 @@
import com.google.gerrit.extensions.events.LifecycleListener;
import com.google.gerrit.lifecycle.LifecycleModule;
+import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.index.ChangeSchemas;
import com.google.gerrit.server.index.IndexCollection;
import com.google.gerrit.server.index.IndexConfig;
@@ -26,6 +27,8 @@
import com.google.inject.Provides;
import com.google.inject.Singleton;
+import org.eclipse.jgit.lib.Config;
+
public class LuceneIndexModule extends LifecycleModule {
private final Integer singleVersion;
private final int threads;
@@ -44,7 +47,6 @@
@Override
protected void configure() {
- bind(IndexConfig.class).toInstance(IndexConfig.createDefault());
factory(LuceneChangeIndex.Factory.class);
install(new IndexModule(threads));
if (singleVersion == null && base == null) {
@@ -54,6 +56,12 @@
}
}
+ @Provides
+ @Singleton
+ IndexConfig getIndexConfig(@GerritServerConfig Config cfg) {
+ return IndexConfig.fromConfig(cfg);
+ }
+
private static class MultiVersionModule extends LifecycleModule {
@Override
public void configure() {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexConfig.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexConfig.java
index 1857e55..08f4748 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexConfig.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexConfig.java
@@ -18,6 +18,8 @@
import com.google.auto.value.AutoValue;
+import org.eclipse.jgit.lib.Config;
+
/**
* Implementation-specific configuration for secondary indexes.
* <p>
@@ -28,13 +30,30 @@
@AutoValue
public abstract class IndexConfig {
public static IndexConfig createDefault() {
- return create(Integer.MAX_VALUE);
+ return create(0, 0);
}
- public static IndexConfig create(int maxLimit) {
- checkArgument(maxLimit > 0, "maxLimit must be positive: %s", maxLimit);
- return new AutoValue_IndexConfig(maxLimit);
+ public static IndexConfig fromConfig(Config cfg) {
+ return create(
+ cfg.getInt("index", null, "maxLimit", 0),
+ cfg.getInt("index", null, "maxPages", 0));
+ }
+
+ public static IndexConfig create(int maxLimit, int maxPages) {
+ return new AutoValue_IndexConfig(
+ checkLimit(maxLimit, "maxLimit"),
+ checkLimit(maxPages, "maxPages"));
+ }
+
+ private static int checkLimit(int limit, String name) {
+ if (limit == 0) {
+ return Integer.MAX_VALUE;
+ }
+ checkArgument(limit > 0, "%s must be positive: %s", name, limit);
+ return limit;
}
public abstract int maxLimit();
+
+ public abstract int maxPages();
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java
index 51d971d..6068dd0 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java
@@ -132,6 +132,13 @@
if (limit == getBackendSupportedLimit()) {
limit--;
}
+
+ int page = (start / limit) + 1;
+ if (page > indexConfig.maxPages()) {
+ throw new QueryParseException(
+ "Cannot go beyond page " + indexConfig.maxPages() + "of results");
+ }
+
Predicate<ChangeData> s = queryRewriter.rewrite(q, start, limit + 1);
if (!(s instanceof ChangeDataSource)) {
q = Predicate.and(open(), q);
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index 9ad896e..85e06e8 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -84,9 +84,19 @@
@Ignore
@RunWith(ConfigSuite.class)
public abstract class AbstractQueryChangesTest {
+ @ConfigSuite.Default
+ public static Config defaultConfig() {
+ return updateConfig(new Config());
+ }
+
@ConfigSuite.Config
public static Config noteDbEnabled() {
- return NotesMigration.allEnabledConfig();
+ return updateConfig(NotesMigration.allEnabledConfig());
+ }
+
+ private static Config updateConfig(Config cfg) {
+ cfg.setInt("index", null, "maxPages", 10);
+ return cfg;
}
@ConfigSuite.Parameter public Config config;
@@ -557,6 +567,19 @@
}
@Test
+ public void maxPages() throws Exception {
+ TestRepository<InMemoryRepository> repo = createProject("repo");
+ Change change = newChange(repo, null, null, null, null).insert();
+
+ QueryRequest query = newQuery("status:new").withLimit(10);
+ assertQuery(query, change);
+ assertQuery(query.withStart(1));
+ assertQuery(query.withStart(99));
+ assertBadQuery(query.withStart(100));
+ assertQuery(query.withLimit(100).withStart(100));
+ }
+
+ @Test
public void updateOrder() throws Exception {
clockStepMs = MILLISECONDS.convert(2, MINUTES);
TestRepository<InMemoryRepository> repo = createProject("repo");
@@ -1073,8 +1096,12 @@
}
protected void assertBadQuery(Object query) throws Exception {
+ assertBadQuery(newQuery(query));
+ }
+
+ protected void assertBadQuery(QueryRequest query) throws Exception {
try {
- newQuery(query).get();
+ query.get();
fail("expected BadRequestException for query: " + query);
} catch (BadRequestException e) {
// Expected.
diff --git a/gerrit-solr/src/main/java/com/google/gerrit/solr/SolrIndexModule.java b/gerrit-solr/src/main/java/com/google/gerrit/solr/SolrIndexModule.java
index 38de6ee..0133e33 100644
--- a/gerrit-solr/src/main/java/com/google/gerrit/solr/SolrIndexModule.java
+++ b/gerrit-solr/src/main/java/com/google/gerrit/solr/SolrIndexModule.java
@@ -50,7 +50,6 @@
@Override
protected void configure() {
- bind(IndexConfig.class).toInstance(IndexConfig.createDefault());
install(new IndexModule(threads));
bind(ChangeIndex.class).to(SolrChangeIndex.class);
listener().to(SolrChangeIndex.class);
@@ -61,6 +60,12 @@
@Provides
@Singleton
+ IndexConfig getIndexConfig(@GerritServerConfig Config cfg) {
+ return IndexConfig.fromConfig(cfg);
+ }
+
+ @Provides
+ @Singleton
public SolrChangeIndex getChangeIndex(@GerritServerConfig Config cfg,
Provider<ReviewDb> db,
ChangeData.Factory changeDataFactory,
