summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDiego Zambelli Sessona <diego.sessona@gmail.com>2023-07-10 14:22:29 +0100
committerDiego Zambelli Sessona <diego.sessona@gmail.com>2023-07-10 14:22:29 +0100
commita0da72341b8159c5341af711e6b8991ef1aa7875 (patch)
treeccaa2aacdbbb44153960809bd1ca8d6e68b5e71d
parent3bc24131ee6f62c89509f318665073715457dc05 (diff)
Add configuration to disable indexes pagination
Add NONE option to disable index backend pagination, allowing the user deciding between performance and data consistency. This option needs to be honoured by the indexing backend and this change introduces the correct implementation for Lucene. We should also create the corresponding change on the elasticsearch libModule for throwing an IAE to highlight that the backend doesn't support NONE pagination type. API pagination is not affected by this change and is always preserved. When having pagination, we could encounter some problems in the result set, when moving from one page to the next of the index results: * duplicated entries (when new changes are created) * missing entries (when changes are removed or set to private) * wrong entries status (when changes are switching their state during the page switching) Having duplicates or missing results could have side effects, depending on the use-case of the consumer of the data. For example, since indexes are used to populate some LoadingCache (SearchingChangeCache [1]), inconsistent results returned by the indexing backend may cause the cache loading function to fail or keep an inconsistent state which doesn't reflect the underlying data. For this case, a workaround [2] has been put in place in master and 3.8, however, it will not work for the missing or inconsistent entries. [1]: https://gerrit.googlesource.com/gerrit/+/refs/heads/stable-3.6/java/com/google/gerrit/server/git/SearchingChangeCacheImpl.java#157 [2]: https://gerrit-review.googlesource.com/c/gerrit/+/349995 Bug: Issue 287484350 Release-Notes: Introduce NONE pagination type and its support in Lucene indexing backend Change-Id: I40f87895d9dac951ae30c5562b2ddbf28b34a41a Co-Authored-With: Fabio Ponciroli <ponch78@gmail.com>
-rw-r--r--Documentation/config-gerrit.txt18
-rw-r--r--java/com/google/gerrit/index/PaginationType.java5
-rw-r--r--java/com/google/gerrit/index/query/PaginatingSource.java8
-rw-r--r--java/com/google/gerrit/index/query/QueryProcessor.java7
-rw-r--r--java/com/google/gerrit/index/testing/AbstractFakeIndex.java15
-rw-r--r--java/com/google/gerrit/lucene/AbstractLuceneIndex.java10
-rw-r--r--java/com/google/gerrit/lucene/LuceneChangeIndex.java6
-rw-r--r--javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java7
-rw-r--r--javatests/com/google/gerrit/server/query/change/FakeQueryChangesLatestIndexVersionTest.java7
-rw-r--r--javatests/com/google/gerrit/server/query/change/FakeQueryChangesPreviousIndexVersionTest.java7
-rw-r--r--javatests/com/google/gerrit/server/query/change/FakeQueryChangesTest.java140
-rw-r--r--javatests/com/google/gerrit/server/query/change/LuceneQueryChangesLatestIndexVersionTest.java7
-rw-r--r--javatests/com/google/gerrit/server/query/change/LuceneQueryChangesPreviousIndexVersionTest.java7
-rw-r--r--javatests/com/google/gerrit/server/query/group/AbstractFakeQueryGroupsTest.java80
-rw-r--r--javatests/com/google/gerrit/server/query/group/BUILD6
-rw-r--r--javatests/com/google/gerrit/server/query/group/FakeQueryGroupsTest.java16
16 files changed, 324 insertions, 22 deletions
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index a6c7da6778..682239f35d 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -3253,6 +3253,8 @@ obtain the next set of results. Supported values are:
+
Index queries are repeated with a non-zero offset to obtain the
next set of results.
+_Note: Results may be inaccurate if the data-set is changing during the query
+execution._
+
* `SEARCH_AFTER`
+
@@ -3260,6 +3262,18 @@ Index queries are repeated using a search-after object. Index
backends can provide their custom implementations for search-after.
Note that, `SEARCH_AFTER` does not impact using offsets in Gerrit
query APIs.
+_Note: Depending on the index backend and its settings, results may be
+inaccurate if the data-set is changing during the query execution._
++
+* `NONE`
++
+Index queries are executed returning all results, without internal
+pagination.
+_Note: Since the entire set of indexing results is kept in memory
+during the API execution, this option may lead to higher memory utilisation
+and overall reduced performance.
+Bear in mind that some indexing backends may not support unbounded queries;
+therefore, the NONE option is unavailable._
+
Defaults to `OFFSET`.
@@ -3305,6 +3319,8 @@ likely help reduce the impacts of this.
For example, if the limit of the previous query was 500 and pageSizeMultiplier
is configured to 5, the next query will have a limit of 2500.
+
+_Note: ignored when paginationType is `NONE`_
++
Defaults to 1 which effectively turns this feature off.
[[index.maxPageSize]]index.maxPageSize::
@@ -3317,6 +3333,8 @@ For example, if the limit of previous query was 500, pageSizeMultiplier is
configured to 5 and maxPageSize to 2000, the next query will have a limit of
2000 (instead of 2500).
+
+_Note: ignored when paginationType is `NONE`_
++
Defaults to no limit.
[[index.maxTerms]]index.maxTerms::
diff --git a/java/com/google/gerrit/index/PaginationType.java b/java/com/google/gerrit/index/PaginationType.java
index e7e34fdc0a..f18f2899a5 100644
--- a/java/com/google/gerrit/index/PaginationType.java
+++ b/java/com/google/gerrit/index/PaginationType.java
@@ -25,5 +25,8 @@ public enum PaginationType {
* <p>For example, Lucene implementation uses the last doc from the previous search as
* search-after object and uses the IndexSearcher.searchAfter API to get the next set of results.
*/
- SEARCH_AFTER
+ SEARCH_AFTER,
+
+ /** Index queries are executed returning all results, without internal pagination. */
+ NONE
}
diff --git a/java/com/google/gerrit/index/query/PaginatingSource.java b/java/com/google/gerrit/index/query/PaginatingSource.java
index b05c8f4846..03d749ac14 100644
--- a/java/com/google/gerrit/index/query/PaginatingSource.java
+++ b/java/com/google/gerrit/index/query/PaginatingSource.java
@@ -63,7 +63,13 @@ public class PaginatingSource<T> implements DataSource<T> {
pageResultSize++;
}
- if (last != null && source instanceof Paginated) {
+ if (last != null
+ && source instanceof Paginated
+ // TODO: this fix is only for the stable branches and the real refactoring would be to
+ // restore the logic
+ // for the filtering in AndSource (L58 - 64) as per
+ // https://gerrit-review.googlesource.com/c/gerrit/+/345634/9
+ && !indexConfig.paginationType().equals(PaginationType.NONE)) {
// Restart source and continue if we have not filled the
// full limit the caller wants.
//
diff --git a/java/com/google/gerrit/index/query/QueryProcessor.java b/java/com/google/gerrit/index/query/QueryProcessor.java
index 21d4c2e7f6..b83c1065bd 100644
--- a/java/com/google/gerrit/index/query/QueryProcessor.java
+++ b/java/com/google/gerrit/index/query/QueryProcessor.java
@@ -25,6 +25,7 @@ import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Ordering;
import com.google.common.flogger.FluentLogger;
+import com.google.common.primitives.Ints;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.index.Index;
@@ -265,7 +266,11 @@ public abstract class QueryProcessor<T> {
start,
initialPageSize,
pageSizeMultiplier,
- limit,
+ // Always bump limit by 1, even if this results in exceeding the permitted
+ // max for this user. The only way to see if there are more entities is to
+ // ask for one more result from the query.
+ // NOTE: This is consistent to the behaviour before the introduction of pagination.`
+ Ints.saturatedCast((long) limit + 1),
getRequestedFields());
logger.atFine().log("Query options: %s", opts);
Predicate<T> pred = rewriter.rewrite(q, opts);
diff --git a/java/com/google/gerrit/index/testing/AbstractFakeIndex.java b/java/com/google/gerrit/index/testing/AbstractFakeIndex.java
index 4241828b8a..8ecdae86e6 100644
--- a/java/com/google/gerrit/index/testing/AbstractFakeIndex.java
+++ b/java/com/google/gerrit/index/testing/AbstractFakeIndex.java
@@ -16,6 +16,7 @@ package com.google.gerrit.index.testing;
import static com.google.common.collect.ImmutableList.toImmutableList;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ImmutableMap;
@@ -49,6 +50,7 @@ import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import java.time.Instant;
+import java.util.ArrayList;
import java.util.Comparator;
import java.util.HashMap;
import java.util.List;
@@ -74,6 +76,7 @@ public abstract class AbstractFakeIndex<K, V, D> implements Index<K, V> {
private final String indexName;
private final Map<K, D> indexedDocuments;
private int queryCount;
+ private List<Integer> resultsSizes;
AbstractFakeIndex(Schema<V> schema, SitePaths sitePaths, String indexName) {
this.schema = schema;
@@ -81,6 +84,7 @@ public abstract class AbstractFakeIndex<K, V, D> implements Index<K, V> {
this.indexName = indexName;
this.indexedDocuments = new HashMap<>();
this.queryCount = 0;
+ this.resultsSizes = new ArrayList<Integer>();
}
@Override
@@ -118,6 +122,16 @@ public abstract class AbstractFakeIndex<K, V, D> implements Index<K, V> {
return queryCount;
}
+ @VisibleForTesting
+ public void resetQueryCount() {
+ queryCount = 0;
+ }
+
+ @VisibleForTesting
+ public List<Integer> getResultsSizes() {
+ return resultsSizes;
+ }
+
@Override
public DataSource<V> getSource(Predicate<V> p, QueryOptions opts) {
List<V> results;
@@ -141,6 +155,7 @@ public abstract class AbstractFakeIndex<K, V, D> implements Index<K, V> {
results = valueStream.skip(opts.start()).limit(opts.pageSize()).collect(toImmutableList());
}
queryCount++;
+ resultsSizes.add(results.size());
}
return new DataSource<>() {
@Override
diff --git a/java/com/google/gerrit/lucene/AbstractLuceneIndex.java b/java/com/google/gerrit/lucene/AbstractLuceneIndex.java
index 586887b1e6..ca81966fea 100644
--- a/java/com/google/gerrit/lucene/AbstractLuceneIndex.java
+++ b/java/com/google/gerrit/lucene/AbstractLuceneIndex.java
@@ -25,6 +25,7 @@ import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Sets;
import com.google.common.flogger.FluentLogger;
+import com.google.common.primitives.Ints;
import com.google.common.util.concurrent.AbstractFuture;
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.ListenableFuture;
@@ -36,6 +37,7 @@ import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.index.FieldDef;
import com.google.gerrit.index.FieldType;
import com.google.gerrit.index.Index;
+import com.google.gerrit.index.PaginationType;
import com.google.gerrit.index.QueryOptions;
import com.google.gerrit.index.Schema;
import com.google.gerrit.index.Schema.Values;
@@ -420,6 +422,10 @@ public abstract class AbstractLuceneIndex<K, V> implements Index<K, V> {
return f.isStored() ? Field.Store.YES : Field.Store.NO;
}
+ static int getLimitBasedOnPaginationType(QueryOptions opts, int pagesize) {
+ return PaginationType.NONE == opts.config().paginationType() ? opts.limit() : pagesize;
+ }
+
private final class NrtFuture extends AbstractFuture<Void> {
private final long gen;
@@ -539,7 +545,9 @@ public abstract class AbstractLuceneIndex<K, V> implements Index<K, V> {
ScoreDoc scoreDoc = null;
try {
searcher = acquire();
- int realLimit = opts.start() + opts.pageSize();
+ int realLimit =
+ Ints.saturatedCast(
+ (long) getLimitBasedOnPaginationType(opts, opts.pageSize()) + opts.start());
TopFieldDocs docs =
opts.searchAfter() != null
? searcher.searchAfter(
diff --git a/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
index daa921ccde..f6f1eda59a 100644
--- a/java/com/google/gerrit/lucene/LuceneChangeIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
@@ -430,6 +430,7 @@ public class LuceneChangeIndex implements ChangeIndex {
if (Integer.MAX_VALUE - opts.pageSize() < opts.start()) {
realPageSize = Integer.MAX_VALUE;
}
+ int queryLimit = AbstractLuceneIndex.getLimitBasedOnPaginationType(opts, realPageSize);
List<TopFieldDocs> hits = new ArrayList<>();
int searchAfterHitsCount = 0;
for (int i = 0; i < indexes.size(); i++) {
@@ -453,11 +454,10 @@ public class LuceneChangeIndex implements ChangeIndex {
subIndex, Iterables.getLast(Arrays.asList(subIndexHits.scoreDocs), searchAfter));
}
} else {
- hits.add(searchers[i].search(query, realPageSize, sort));
+ hits.add(searchers[i].search(query, queryLimit, sort));
}
}
- TopDocs docs =
- TopDocs.merge(sort, realPageSize, hits.stream().toArray(TopFieldDocs[]::new));
+ TopDocs docs = TopDocs.merge(sort, queryLimit, hits.stream().toArray(TopFieldDocs[]::new));
List<Document> result = new ArrayList<>(docs.scoreDocs.length);
for (int i = opts.start(); i < docs.scoreDocs.length; i++) {
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index 081fe02758..a820f86c2b 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -95,6 +95,7 @@ import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.httpd.raw.IndexPreloadingUtil;
import com.google.gerrit.index.FieldDef;
import com.google.gerrit.index.IndexConfig;
+import com.google.gerrit.index.PaginationType;
import com.google.gerrit.index.Schema;
import com.google.gerrit.index.query.IndexPredicate;
import com.google.gerrit.index.query.Predicate;
@@ -118,6 +119,7 @@ import com.google.gerrit.server.change.ChangeInserter;
import com.google.gerrit.server.change.ChangeTriplet;
import com.google.gerrit.server.change.NotifyResolver;
import com.google.gerrit.server.change.PatchSetInserter;
+import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.experiments.ExperimentFeaturesConstants;
import com.google.gerrit.server.git.meta.MetaDataUpdate;
@@ -179,6 +181,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
@Inject protected AccountManager accountManager;
@Inject protected AllUsersName allUsersName;
@Inject protected BatchUpdate.Factory updateFactory;
+ @Inject protected AllProjectsName allProjectsName;
@Inject protected ChangeInserter.Factory changeFactory;
@Inject protected Provider<ChangeQueryBuilder> queryBuilderProvider;
@Inject protected GerritApi gApi;
@@ -4635,4 +4638,8 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
protected Schema<ChangeData> getSchema() {
return indexes.getSearchIndex().getSchema();
}
+
+ PaginationType getCurrentPaginationType() {
+ return config.getEnum("index", null, "paginationType", PaginationType.OFFSET);
+ }
}
diff --git a/javatests/com/google/gerrit/server/query/change/FakeQueryChangesLatestIndexVersionTest.java b/javatests/com/google/gerrit/server/query/change/FakeQueryChangesLatestIndexVersionTest.java
index 4dde452dc9..df6ff5a15e 100644
--- a/javatests/com/google/gerrit/server/query/change/FakeQueryChangesLatestIndexVersionTest.java
+++ b/javatests/com/google/gerrit/server/query/change/FakeQueryChangesLatestIndexVersionTest.java
@@ -33,4 +33,11 @@ public class FakeQueryChangesLatestIndexVersionTest extends FakeQueryChangesTest
config.setString("index", null, "paginationType", "SEARCH_AFTER");
return config;
}
+
+ @ConfigSuite.Config
+ public static Config nonePaginationType() {
+ Config config = defaultConfig();
+ config.setString("index", null, "paginationType", "NONE");
+ return config;
+ }
}
diff --git a/javatests/com/google/gerrit/server/query/change/FakeQueryChangesPreviousIndexVersionTest.java b/javatests/com/google/gerrit/server/query/change/FakeQueryChangesPreviousIndexVersionTest.java
index 95896dc991..805c99abdb 100644
--- a/javatests/com/google/gerrit/server/query/change/FakeQueryChangesPreviousIndexVersionTest.java
+++ b/javatests/com/google/gerrit/server/query/change/FakeQueryChangesPreviousIndexVersionTest.java
@@ -43,4 +43,11 @@ public class FakeQueryChangesPreviousIndexVersionTest extends FakeQueryChangesTe
config.setString("index", null, "paginationType", "SEARCH_AFTER");
return config;
}
+
+ @ConfigSuite.Config
+ public static Config nonePaginationType() {
+ Config config = againstPreviousIndexVersion();
+ config.setString("index", null, "paginationType", "NONE");
+ return config;
+ }
}
diff --git a/javatests/com/google/gerrit/server/query/change/FakeQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/FakeQueryChangesTest.java
index 3968a33707..d20a00465c 100644
--- a/javatests/com/google/gerrit/server/query/change/FakeQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/FakeQueryChangesTest.java
@@ -15,16 +15,24 @@
package com.google.gerrit.server.query.change;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allow;
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allowCapability;
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.block;
import static com.google.gerrit.common.data.GlobalCapability.QUERY_LIMIT;
+import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
+import static org.junit.Assume.assumeFalse;
+import static org.junit.Assume.assumeTrue;
import com.google.gerrit.acceptance.UseClockStep;
import com.google.gerrit.entities.Permission;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.index.PaginationType;
+import com.google.gerrit.index.query.QueryParseException;
import com.google.gerrit.index.testing.AbstractFakeIndex;
import com.google.gerrit.server.config.AllProjectsName;
+import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.index.change.ChangeIndexCollection;
import com.google.gerrit.testing.InMemoryModule;
import com.google.gerrit.testing.InMemoryRepositoryManager;
@@ -32,6 +40,7 @@ import com.google.gerrit.testing.InMemoryRepositoryManager.Repo;
import com.google.inject.Guice;
import com.google.inject.Inject;
import com.google.inject.Injector;
+import java.util.List;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Config;
import org.junit.Test;
@@ -74,62 +83,167 @@ public abstract class FakeQueryChangesTest extends AbstractQueryChangesTest {
AbstractFakeIndex idx = (AbstractFakeIndex) changeIndexCollection.getSearchIndex();
newQuery("status:new").withLimit(5).get();
- // Since the limit of the query (i.e. 5) is more than the total number of changes (i.e. 4),
- // only 1 index search is expected.
- assertThat(idx.getQueryCount()).isEqualTo(1);
+ assertThatSearchQueryWasNotPaginated(idx.getQueryCount());
}
@Test
@UseClockStep
@SuppressWarnings("unchecked")
public void noLimitQueryPaginates() throws Exception {
+ assumeFalse(PaginationType.NONE == getCurrentPaginationType());
+
TestRepository<InMemoryRepositoryManager.Repo> testRepo = createProject("repo");
// create 4 changes
insert(testRepo, newChange(testRepo));
insert(testRepo, newChange(testRepo));
insert(testRepo, newChange(testRepo));
insert(testRepo, newChange(testRepo));
-
// Set queryLimit to 2
projectOperations
.project(allProjects)
.forUpdate()
.add(allowCapability(QUERY_LIMIT).group(REGISTERED_USERS).range(0, 2))
.update();
-
AbstractFakeIndex idx = (AbstractFakeIndex) changeIndexCollection.getSearchIndex();
-
// 2 index searches are expected. The first index search will run with size 3 (i.e.
// the configured query-limit+1), and then we will paginate to get the remaining
// changes with the second index search.
newQuery("status:new").withNoLimit().get();
- assertThat(idx.getQueryCount()).isEqualTo(2);
+ assertThatSearchQueryWasPaginated(idx.getQueryCount(), 2);
+ }
+
+ @Test
+ @UseClockStep
+ public void noLimitQueryDoesNotPaginatesWithNonePaginationType() throws Exception {
+ assumeTrue(PaginationType.NONE == getCurrentPaginationType());
+ AbstractFakeIndex idx = setupRepoWithFourChanges();
+ newQuery("status:new").withNoLimit().get();
+ assertThatSearchQueryWasNotPaginated(idx.getQueryCount());
+ }
+
+ @Test
+ @UseClockStep
+ public void invisibleChangesNotPaginatedWithNonePaginationType() throws Exception {
+ assumeTrue(PaginationType.NONE == getCurrentPaginationType());
+ AbstractFakeIndex idx = setupRepoWithFourChanges();
+ final int LIMIT = 3;
+
+ projectOperations
+ .project(allProjectsName)
+ .forUpdate()
+ .removeAllAccessSections()
+ .add(allow(Permission.READ).ref("refs/*").group(SystemGroupBackend.REGISTERED_USERS))
+ .update();
+
+ // Set queryLimit to 3
+ projectOperations
+ .project(allProjects)
+ .forUpdate()
+ .add(allowCapability(QUERY_LIMIT).group(ANONYMOUS_USERS).range(0, LIMIT))
+ .update();
+
+ requestContext.setContext(anonymousUserProvider::get);
+ List<ChangeInfo> result = newQuery("status:new").withLimit(LIMIT).get();
+ assertThat(result.size()).isEqualTo(0);
+ assertThatSearchQueryWasNotPaginated(idx.getQueryCount());
+ assertThat(idx.getResultsSizes().get(0)).isEqualTo(LIMIT + 1);
+ }
+
+ @Test
+ @UseClockStep
+ public void invisibleChangesPaginatedWithPagination() throws Exception {
+ assumeFalse(PaginationType.NONE == getCurrentPaginationType());
+
+ AbstractFakeIndex idx = setupRepoWithFourChanges();
+ final int LIMIT = 3;
+
+ projectOperations
+ .project(allProjectsName)
+ .forUpdate()
+ .removeAllAccessSections()
+ .add(allow(Permission.READ).ref("refs/*").group(SystemGroupBackend.REGISTERED_USERS))
+ .update();
+
+ projectOperations
+ .project(allProjects)
+ .forUpdate()
+ .add(allowCapability(QUERY_LIMIT).group(ANONYMOUS_USERS).range(0, LIMIT))
+ .update();
+
+ requestContext.setContext(anonymousUserProvider::get);
+ List<ChangeInfo> result = newQuery("status:new").withLimit(LIMIT).get();
+ assertThat(result.size()).isEqualTo(0);
+ assertThatSearchQueryWasPaginated(idx.getQueryCount(), 2);
+ assertThat(idx.getResultsSizes().get(0)).isEqualTo(LIMIT + 1);
+ assertThat(idx.getResultsSizes().get(1)).isEqualTo(0); // Second query size
}
@Test
@UseClockStep
@SuppressWarnings("unchecked")
public void internalQueriesPaginate() throws Exception {
+ assumeFalse(PaginationType.NONE == getCurrentPaginationType());
+ final int LIMIT = 2;
+
+ TestRepository<Repo> testRepo = createProject("repo");
// create 4 changes
- TestRepository<InMemoryRepositoryManager.Repo> testRepo = createProject("repo");
insert(testRepo, newChange(testRepo));
insert(testRepo, newChange(testRepo));
insert(testRepo, newChange(testRepo));
insert(testRepo, newChange(testRepo));
-
// Set queryLimit to 2
projectOperations
.project(allProjects)
.forUpdate()
- .add(allowCapability(QUERY_LIMIT).group(REGISTERED_USERS).range(0, 2))
+ .add(allowCapability(QUERY_LIMIT).group(REGISTERED_USERS).range(0, LIMIT))
.update();
-
AbstractFakeIndex idx = (AbstractFakeIndex) changeIndexCollection.getSearchIndex();
-
// 2 index searches are expected. The first index search will run with size 3 (i.e.
// the configured query-limit+1), and then we will paginate to get the remaining
// changes with the second index search.
queryProvider.get().query(queryBuilderProvider.get().parse("status:new"));
- assertThat(idx.getQueryCount()).isEqualTo(2);
+ assertThat(idx.getQueryCount()).isEqualTo(LIMIT);
+ }
+
+ @Test
+ @UseClockStep
+ @SuppressWarnings("unchecked")
+ public void internalQueriesDoNotPaginateWithNonePaginationType() throws Exception {
+ assumeTrue(PaginationType.NONE == getCurrentPaginationType());
+
+ AbstractFakeIndex idx = setupRepoWithFourChanges();
+ // 1 index search is expected since we are not paginating.
+ executeQuery("status:new");
+ assertThatSearchQueryWasNotPaginated(idx.getQueryCount());
+ }
+
+ private void executeQuery(String query) throws QueryParseException {
+ queryProvider.get().query(queryBuilderProvider.get().parse(query));
+ }
+
+ private void assertThatSearchQueryWasNotPaginated(int queryCount) {
+ assertThat(queryCount).isEqualTo(1);
+ }
+
+ private void assertThatSearchQueryWasPaginated(int queryCount, int expectedPages) {
+ assertThat(queryCount).isEqualTo(expectedPages);
+ }
+
+ private AbstractFakeIndex setupRepoWithFourChanges() throws Exception {
+ TestRepository<Repo> testRepo = createProject("repo");
+ // create 4 changes
+ insert(testRepo, newChange(testRepo));
+ insert(testRepo, newChange(testRepo));
+ insert(testRepo, newChange(testRepo));
+ insert(testRepo, newChange(testRepo));
+
+ // Set queryLimit to 2
+ projectOperations
+ .project(allProjects)
+ .forUpdate()
+ .add(allowCapability(QUERY_LIMIT).group(REGISTERED_USERS).range(0, 2))
+ .update();
+
+ return (AbstractFakeIndex) changeIndexCollection.getSearchIndex();
}
}
diff --git a/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesLatestIndexVersionTest.java b/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesLatestIndexVersionTest.java
index 45879433e6..bfd1bd6baa 100644
--- a/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesLatestIndexVersionTest.java
+++ b/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesLatestIndexVersionTest.java
@@ -30,4 +30,11 @@ public class LuceneQueryChangesLatestIndexVersionTest extends LuceneQueryChanges
config.setString("index", null, "paginationType", "SEARCH_AFTER");
return config;
}
+
+ @ConfigSuite.Config
+ public static Config nonePaginationType() {
+ Config config = defaultConfig();
+ config.setString("index", null, "paginationType", "NONE");
+ return config;
+ }
}
diff --git a/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesPreviousIndexVersionTest.java b/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesPreviousIndexVersionTest.java
index 178269730c..f93a2a7492 100644
--- a/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesPreviousIndexVersionTest.java
+++ b/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesPreviousIndexVersionTest.java
@@ -40,4 +40,11 @@ public class LuceneQueryChangesPreviousIndexVersionTest extends LuceneQueryChang
config.setString("index", null, "paginationType", "SEARCH_AFTER");
return config;
}
+
+ @ConfigSuite.Config
+ public static Config nonePaginationType() {
+ Config config = againstPreviousIndexVersion();
+ config.setString("index", null, "paginationType", "NONE");
+ return config;
+ }
}
diff --git a/javatests/com/google/gerrit/server/query/group/AbstractFakeQueryGroupsTest.java b/javatests/com/google/gerrit/server/query/group/AbstractFakeQueryGroupsTest.java
new file mode 100644
index 0000000000..bf224f063a
--- /dev/null
+++ b/javatests/com/google/gerrit/server/query/group/AbstractFakeQueryGroupsTest.java
@@ -0,0 +1,80 @@
+// Copyright (C) 2023 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.server.query.group;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assume.assumeTrue;
+
+import com.google.gerrit.extensions.common.GroupInfo;
+import com.google.gerrit.index.PaginationType;
+import com.google.gerrit.index.testing.AbstractFakeIndex;
+import com.google.gerrit.server.index.group.GroupIndexCollection;
+import com.google.gerrit.testing.InMemoryModule;
+import com.google.inject.Guice;
+import com.google.inject.Inject;
+import com.google.inject.Injector;
+import java.util.ArrayList;
+import java.util.List;
+import org.eclipse.jgit.lib.Config;
+import org.junit.Before;
+import org.junit.Test;
+
+public abstract class AbstractFakeQueryGroupsTest extends AbstractQueryGroupsTest {
+
+ @Inject private GroupIndexCollection groupIndexCollection;
+
+ @Override
+ protected Injector createInjector() {
+ Config fakeConfig = new Config(config);
+ InMemoryModule.setDefaults(fakeConfig);
+ fakeConfig.setString("index", null, "type", "fake");
+ return Guice.createInjector(new InMemoryModule(fakeConfig));
+ }
+
+ @Before
+ public void resetQueryCount() {
+ ((AbstractFakeIndex<?, ?, ?>) groupIndexCollection.getSearchIndex()).resetQueryCount();
+ }
+
+ @Test
+ public void internalQueriesDoNotPaginateWithNonePaginationType() throws Exception {
+ assumeTrue(PaginationType.NONE == getCurrentPaginationType());
+
+ final int GROUPS_CREATED_SIZE = 2;
+ List<GroupInfo> groupsCreated = new ArrayList<>();
+ for (int i = 0; i < GROUPS_CREATED_SIZE; i++) {
+ groupsCreated.add(createGroupThatIsVisibleToAll(name("group-" + i)));
+ }
+
+ List<GroupInfo> result = assertQuery(newQuery("is:visibletoall"), groupsCreated);
+ assertThat(result.size()).isEqualTo(GROUPS_CREATED_SIZE);
+ assertThat(result.get(result.size() - 1)._moreGroups).isNull();
+ assertThatSearchQueryWasNotPaginated();
+ }
+
+ PaginationType getCurrentPaginationType() {
+ return config.getEnum("index", null, "paginationType", PaginationType.OFFSET);
+ }
+
+ private void assertThatSearchQueryWasNotPaginated() {
+ assertThat(getQueryCount()).isEqualTo(1);
+ }
+
+ private int getQueryCount() {
+ AbstractFakeIndex<?, ?, ?> idx =
+ (AbstractFakeIndex<?, ?, ?>) groupIndexCollection.getSearchIndex();
+ return idx.getQueryCount();
+ }
+}
diff --git a/javatests/com/google/gerrit/server/query/group/BUILD b/javatests/com/google/gerrit/server/query/group/BUILD
index 0cc132d53e..0094bd6a93 100644
--- a/javatests/com/google/gerrit/server/query/group/BUILD
+++ b/javatests/com/google/gerrit/server/query/group/BUILD
@@ -1,7 +1,10 @@
load("@rules_java//java:defs.bzl", "java_library")
load("//tools/bzl:junit.bzl", "junit_tests")
-ABSTRACT_QUERY_TEST = ["AbstractQueryGroupsTest.java"]
+ABSTRACT_QUERY_TEST = [
+ "AbstractQueryGroupsTest.java",
+ "AbstractFakeQueryGroupsTest.java",
+]
java_library(
name = "abstract_query_tests",
@@ -13,6 +16,7 @@ java_library(
"//java/com/google/gerrit/entities",
"//java/com/google/gerrit/extensions:api",
"//java/com/google/gerrit/index",
+ "//java/com/google/gerrit/index/testing",
"//java/com/google/gerrit/lifecycle",
"//java/com/google/gerrit/server",
"//java/com/google/gerrit/server/schema",
diff --git a/javatests/com/google/gerrit/server/query/group/FakeQueryGroupsTest.java b/javatests/com/google/gerrit/server/query/group/FakeQueryGroupsTest.java
index e4f228a084..d347716923 100644
--- a/javatests/com/google/gerrit/server/query/group/FakeQueryGroupsTest.java
+++ b/javatests/com/google/gerrit/server/query/group/FakeQueryGroupsTest.java
@@ -25,12 +25,26 @@ import java.util.List;
import java.util.Map;
import org.eclipse.jgit.lib.Config;
-public class FakeQueryGroupsTest extends AbstractQueryGroupsTest {
+public class FakeQueryGroupsTest extends AbstractFakeQueryGroupsTest {
@ConfigSuite.Default
public static Config defaultConfig() {
return IndexConfig.createForFake();
}
+ @ConfigSuite.Config
+ public static Config searchAfterPaginationType() {
+ Config config = defaultConfig();
+ config.setString("index", null, "paginationType", "SEARCH_AFTER");
+ return config;
+ }
+
+ @ConfigSuite.Config
+ public static Config nonePaginationType() {
+ Config config = defaultConfig();
+ config.setString("index", null, "paginationType", "NONE");
+ return config;
+ }
+
@ConfigSuite.Configs
public static Map<String, Config> againstPreviousIndexVersion() {
// the current schema version is already tested by the inherited default config suite