Improve test coverage for index query pagination

Add test to ensure we stop querying index when there are
no more results. This test actually adds coverage for the
fix done in I3143e6935 but could not be added in that
change as the fake index implementation did not allow it.

Release-Notes: skip
Change-Id: I3ce8604893aa11830346d2a04cd3d428a6425c64

3 files changed, 51 insertions, 2 deletions

diff --git a/java/com/google/gerrit/index/testing/AbstractFakeIndex.java b/java/com/google/gerrit/index/testing/AbstractFakeIndex.java
index 5713cbae77..4197c64011 100644
--- a/java/com/google/gerrit/index/testing/AbstractFakeIndex.java
+++ b/java/com/google/gerrit/index/testing/AbstractFakeIndex.java
@@ -73,12 +73,14 @@ public abstract class AbstractFakeIndex<K, V, D> implements Index<K, V> {
 
   private final String indexName;
   private final Map<K, D> indexedDocuments;
+  private int queryCount;
 
   AbstractFakeIndex(Schema<V> schema, SitePaths sitePaths, String indexName) {
     this.schema = schema;
     this.sitePaths = sitePaths;
     this.indexName = indexName;
     this.indexedDocuments = new HashMap<>();
+    this.queryCount = 0;
   }
 
   @Override
@@ -112,6 +114,10 @@ public abstract class AbstractFakeIndex<K, V, D> implements Index<K, V> {
     }
   }
 
+  public int getQueryCount() {
+    return queryCount;
+  }
+
   @Override
   public DataSource<V> getSource(Predicate<V> p, QueryOptions opts) {
     List<V> results;
@@ -134,6 +140,7 @@ public abstract class AbstractFakeIndex<K, V, D> implements Index<K, V> {
       } else {
         results = valueStream.skip(opts.start()).limit(opts.pageSize()).collect(toImmutableList());
       }
+      queryCount++;
     }
     return new DataSource<V>() {
       @Override
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index e9155ce075..c3b220b0e4 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -197,9 +197,9 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   @Inject protected IdentifiedUser.GenericFactory identifiedUserFactory;
   @Inject protected AuthRequest.Factory authRequestFactory;
   @Inject protected ExternalIdFactory externalIdFactory;
+  @Inject protected ProjectOperations projectOperations;
 
   @Inject private ProjectConfig.Factory projectConfigFactory;
-  @Inject private ProjectOperations projectOperations;
 
   protected Injector injector;
   protected LifecycleManager lifecycle;
@@ -718,7 +718,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
     assertThat(thrown).hasMessageThat().contains("invalid value");
   }
 
-  private Change createChange(TestRepository<Repo> repo, PersonIdent person) throws Exception {
+  protected Change createChange(TestRepository<Repo> repo, PersonIdent person) throws Exception {
     RevCommit commit =
         repo.parseBody(repo.commit().message("message").author(person).committer(person).create());
     return insert(repo, newChangeForCommit(repo, commit), null);
diff --git a/javatests/com/google/gerrit/server/query/change/FakeQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/FakeQueryChangesTest.java
index 1e23420b04..ef7a3fbd09 100644
--- a/javatests/com/google/gerrit/server/query/change/FakeQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/FakeQueryChangesTest.java
@@ -14,10 +14,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.block;
+import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
+
+import com.google.gerrit.acceptance.UseClockStep;
+import com.google.gerrit.entities.Permission;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.index.testing.AbstractFakeIndex;
+import com.google.gerrit.server.index.change.ChangeIndexCollection;
 import com.google.gerrit.testing.InMemoryModule;
+import com.google.gerrit.testing.InMemoryRepositoryManager;
+import com.google.gerrit.testing.InMemoryRepositoryManager.Repo;
 import com.google.inject.Guice;
+import com.google.inject.Inject;
 import com.google.inject.Injector;
+import org.eclipse.jgit.junit.TestRepository;
 import org.eclipse.jgit.lib.Config;
+import org.junit.Test;
 
 /**
  * Test against {@link com.google.gerrit.index.testing.AbstractFakeIndex}. This test might seem
@@ -25,6 +39,8 @@ import org.eclipse.jgit.lib.Config;
  * results as production indices.
  */
 public abstract class FakeQueryChangesTest extends AbstractQueryChangesTest {
+  @Inject private ChangeIndexCollection changeIndexCollection;
+
   @Override
   protected Injector createInjector() {
     Config fakeConfig = new Config(config);
@@ -32,4 +48,30 @@ public abstract class FakeQueryChangesTest extends AbstractQueryChangesTest {
     fakeConfig.setString("index", null, "type", "fake");
     return Guice.createInjector(new InMemoryModule(fakeConfig));
   }
+
+  @Test
+  @UseClockStep
+  @SuppressWarnings("unchecked")
+  public void stopQueryIfNoMoreResults() throws Exception {
+    // create 2 visible changes
+    TestRepository<InMemoryRepositoryManager.Repo> testRepo = createProject("repo");
+    insert(testRepo, newChange(testRepo));
+    insert(testRepo, newChange(testRepo));
+
+    // create 2 invisible changes
+    TestRepository<Repo> hiddenProject = createProject("hiddenProject");
+    insert(hiddenProject, newChange(hiddenProject));
+    insert(hiddenProject, newChange(hiddenProject));
+    projectOperations
+        .project(Project.nameKey("hiddenProject"))
+        .forUpdate()
+        .add(block(Permission.READ).ref("refs/*").group(REGISTERED_USERS))
+        .update();
+
+    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);
+  }
 }