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,