Merge branch 'stable-3.7' into stable-3.8
* stable-3.7:
Set version to 3.7.5-SNAPSHOT
Set version to 3.7.4
Set version to 3.6.7-SNAPSHOT
Set version to 3.6.6
Preserve refs order in the GitBatchRefUpdateEvent event
Remove failing PolyGerrit tests on Chrome Headless 112.0.5615.165
Add configuration to disable indexes pagination
DropWizardMetricMaker: fix the javadoc unclosed code block
Release-Notes: skip
Change-Id: I09bdf4a486960c65ac3523bd9af7cb64d5178955
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 0293e66..6b8f10a 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -3324,6 +3324,8 @@
+
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`
+
@@ -3331,6 +3333,18 @@
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`.
@@ -3376,6 +3390,8 @@
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::
@@ -3388,6 +3404,8 @@
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 e7e34fd..f18f289 100644
--- a/java/com/google/gerrit/index/PaginationType.java
+++ b/java/com/google/gerrit/index/PaginationType.java
@@ -25,5 +25,8 @@
* <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 8431ccc6..98a0ed3 100644
--- a/java/com/google/gerrit/index/query/PaginatingSource.java
+++ b/java/com/google/gerrit/index/query/PaginatingSource.java
@@ -63,7 +63,13 @@
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 1c8bbc3..e6077ad 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.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 @@
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);
// Apply index-specific rewrite first
diff --git a/java/com/google/gerrit/index/testing/AbstractFakeIndex.java b/java/com/google/gerrit/index/testing/AbstractFakeIndex.java
index 1d02dab..944f956 100644
--- a/java/com/google/gerrit/index/testing/AbstractFakeIndex.java
+++ b/java/com/google/gerrit/index/testing/AbstractFakeIndex.java
@@ -16,6 +16,7 @@
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;
@@ -50,6 +51,7 @@
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;
@@ -75,6 +77,7 @@
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;
@@ -82,6 +85,7 @@
this.indexName = indexName;
this.indexedDocuments = new HashMap<>();
this.queryCount = 0;
+ this.resultsSizes = new ArrayList<Integer>();
}
@Override
@@ -119,6 +123,16 @@
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;
@@ -142,6 +156,7 @@
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 f9dc31a..2fd1c45 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.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;
@@ -35,6 +36,7 @@
import com.google.gerrit.exceptions.StorageException;
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;
@@ -422,6 +424,10 @@
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;
@@ -541,7 +547,9 @@
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 bc614d8..4c05f70 100644
--- a/java/com/google/gerrit/lucene/LuceneChangeIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneChangeIndex.java
@@ -404,6 +404,7 @@
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++) {
@@ -427,11 +428,10 @@
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/java/com/google/gerrit/metrics/dropwizard/DropWizardMetricMaker.java b/java/com/google/gerrit/metrics/dropwizard/DropWizardMetricMaker.java
index 0821e86..2956a3e 100644
--- a/java/com/google/gerrit/metrics/dropwizard/DropWizardMetricMaker.java
+++ b/java/com/google/gerrit/metrics/dropwizard/DropWizardMetricMaker.java
@@ -370,15 +370,16 @@
}
/**
- * Ensures that the sanitized metric name doesn't contain invalid characters and
- * removes the risk of collision (between the sanitized metric names).
- * Modifications to the input metric name:
+ * Ensures that the sanitized metric name doesn't contain invalid characters and removes the risk
+ * of collision (between the sanitized metric names). Modifications to the input metric name:
+ *
* <ul>
- * <li/> leading <code>/</code> is replaced with <code>_</code>
- * <li/> doubled (or repeated more times) <code>/</code> are reduced to a single <code>/<code>
- * <li/> ending <code>/</code> is removed
- * <li/> all characters that are not <code>/a-zA-Z0-9_-</code> are replaced with <code>_0x[HEX CODE]_</code> (code is capitalized)
- * <li/> the replacement prefix <code>_0x</code> is prepended with another replacement prefix
+ * <li/>leading <code>/</code> is replaced with <code>_</code>
+ * <li/>doubled (or repeated more times) <code>/</code> are reduced to a single <code>/</code>
+ * <li/>ending <code>/</code> is removed
+ * <li/>all characters that are not <code>/a-zA-Z0-9_-</code> are replaced with <code>
+ * _0x[HEX CODE]_</code> (code is capitalized)
+ * <li/>the replacement prefix <code>_0x</code> is prepended with another replacement prefix
* </ul>
*
* @param name name of the metric to sanitize
diff --git a/java/com/google/gerrit/server/extensions/events/GitReferenceUpdated.java b/java/com/google/gerrit/server/extensions/events/GitReferenceUpdated.java
index 814390b..107e987 100644
--- a/java/com/google/gerrit/server/extensions/events/GitReferenceUpdated.java
+++ b/java/com/google/gerrit/server/extensions/events/GitReferenceUpdated.java
@@ -23,7 +23,7 @@
import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.inject.Inject;
import com.google.inject.Singleton;
-import java.util.HashSet;
+import java.util.LinkedHashSet;
import java.util.Set;
import java.util.stream.Collectors;
import org.eclipse.jgit.lib.BatchRefUpdate;
@@ -128,7 +128,7 @@
if (batchRefUpdateListeners.isEmpty() && refUpdatedListeners.isEmpty()) {
return;
}
- Set<GitBatchRefUpdateListener.UpdatedRef> updates = new HashSet<>();
+ Set<GitBatchRefUpdateListener.UpdatedRef> updates = new LinkedHashSet<>();
for (ReceiveCommand cmd : batchRefUpdate.getCommands()) {
if (cmd.getResult() == ReceiveCommand.Result.OK) {
updates.add(
@@ -254,7 +254,7 @@
public Set<String> getRefNames() {
return updatedRefs.stream()
.map(GitBatchRefUpdateListener.UpdatedRef::getRefName)
- .collect(Collectors.toSet());
+ .collect(Collectors.toCollection(LinkedHashSet::new));
}
@Override
diff --git a/javatests/com/google/gerrit/server/extensions/events/GitReferenceUpdatedTest.java b/javatests/com/google/gerrit/server/extensions/events/GitReferenceUpdatedTest.java
index 7bdb23c..8ff6825 100644
--- a/javatests/com/google/gerrit/server/extensions/events/GitReferenceUpdatedTest.java
+++ b/javatests/com/google/gerrit/server/extensions/events/GitReferenceUpdatedTest.java
@@ -14,12 +14,15 @@
package com.google.gerrit.server.extensions.events;
+import static com.google.common.truth.Truth.assertThat;
+
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.events.GitBatchRefUpdateListener;
import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.extensions.events.GitReferenceUpdated.GitBatchRefUpdateEvent;
import com.google.gerrit.server.plugincontext.PluginContext.PluginMetrics;
import com.google.gerrit.server.plugincontext.PluginSetContext;
import java.io.IOException;
@@ -34,6 +37,8 @@
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Captor;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.junit.MockitoJUnitRunner;
@@ -49,6 +54,7 @@
@Mock GitReferenceUpdatedListener refUpdatedListener;
@Mock GitBatchRefUpdateListener batchRefUpdateListener;
@Mock EventUtil util;
+ @Captor ArgumentCaptor<GitBatchRefUpdateEvent> eventCaptor;
@Before
public void setup() {
@@ -87,6 +93,36 @@
}
@Test
+ public void shouldPreserveRefsOrderInTheBatchRefsUpdatedEvent() {
+ BatchRefUpdate update = newBatchRefUpdate();
+ ReceiveCommand cmd1 =
+ new ReceiveCommand(
+ ObjectId.zeroId(),
+ ObjectId.fromString("0000000000000000000000000000000000000001"),
+ "refs/changes/01/1/1");
+ ReceiveCommand cmd2 =
+ new ReceiveCommand(
+ ObjectId.zeroId(),
+ ObjectId.fromString("0000000000000000000000000000000000000001"),
+ "refs/changes/01/1/meta");
+ cmd1.setResult(ReceiveCommand.Result.OK);
+ cmd2.setResult(ReceiveCommand.Result.OK);
+ update.addCommand(cmd1);
+ update.addCommand(cmd2);
+
+ GitReferenceUpdated event =
+ new GitReferenceUpdated(
+ new PluginSetContext<>(batchRefUpdateListeners, PluginMetrics.DISABLED_INSTANCE),
+ new PluginSetContext<>(refUpdatedListeners, PluginMetrics.DISABLED_INSTANCE),
+ util);
+ event.fire(Project.NameKey.parse("project"), update, updater);
+ Mockito.verify(batchRefUpdateListener, Mockito.times(1))
+ .onGitBatchRefUpdate(eventCaptor.capture());
+ GitBatchRefUpdateEvent batchEvent = eventCaptor.getValue();
+ assertThat(batchEvent.getRefNames()).isInOrder();
+ }
+
+ @Test
public void RefUpdateEventAndRefsUpdateEventAreFired_RefUpdate() throws Exception {
String ref = "refs/heads/master";
RefUpdate update = newRefUpdate(ref);
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index a188251..2d7ed10 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.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.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.git.GitRepositoryManager;
import com.google.gerrit.server.git.meta.MetaDataUpdate;
@@ -180,6 +182,7 @@
@Inject protected AccountManager accountManager;
@Inject protected AllUsersName allUsersName;
@Inject protected BatchUpdate.Factory updateFactory;
+ @Inject protected AllProjectsName allProjectsName;
@Inject protected ChangeInserter.Factory changeFactory;
@Inject protected ChangeQueryBuilder queryBuilder;
@Inject protected GerritApi gApi;
@@ -4517,4 +4520,8 @@
update.setAllowWriteToNewRef(true);
return update;
}
+
+ 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 4dde452..df6ff5a 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 @@
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 95896dc..805c99a 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 @@
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 6b17bb6..72fc6d2 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.inject.Guice;
@@ -76,22 +84,20 @@
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
public void noLimitQueryPaginates() throws Exception {
+ assumeFalse(PaginationType.NONE == getCurrentPaginationType());
+
try (TestRepository<Repository> testRepo = createAndOpenProject("repo")) {
- // create 4 changes
insert("repo", newChange(testRepo));
insert("repo", newChange(testRepo));
insert("repo", newChange(testRepo));
insert("repo", newChange(testRepo));
}
-
// Set queryLimit to 2
projectOperations
.project(allProjects)
@@ -106,13 +112,126 @@
// 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
public void internalQueriesPaginate() throws Exception {
- // create 4 changes
+ assumeFalse(PaginationType.NONE == getCurrentPaginationType());
+ final int LIMIT = 2;
+
+ try (TestRepository<Repository> testRepo = createAndOpenProject("repo")) {
+ insert("repo", newChange(testRepo));
+ insert("repo", newChange(testRepo));
+ insert("repo", newChange(testRepo));
+ insert("repo", newChange(testRepo));
+ }
+ // Set queryLimit to 2
+ projectOperations
+ .project(allProjects)
+ .forUpdate()
+ .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(queryBuilder.parse("status:new"));
+ 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(queryBuilder.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 {
try (TestRepository<Repository> testRepo = createAndOpenProject("repo")) {
insert("repo", newChange(testRepo));
insert("repo", newChange(testRepo));
@@ -127,14 +246,6 @@
.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.
- List<ChangeData> matches = queryProvider.get().query(queryBuilder.parse("status:new"));
- assertThat(matches).hasSize(4);
- assertThat(idx.getQueryCount()).isEqualTo(2);
+ 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 4587943..bfd1bd6 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 @@
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 1782697..f93a2a7 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 @@
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 0000000..bf224f0
--- /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 e877c81..550cb41 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",
@@ -14,6 +17,7 @@
"//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 e4f228a..d347716 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.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
