diff options
5 files changed, 64 insertions, 38 deletions
diff --git a/java/com/google/gerrit/index/query/IndexedQuery.java b/java/com/google/gerrit/index/query/IndexedQuery.java index df14e295cc..4bc8043ff7 100644 --- a/java/com/google/gerrit/index/query/IndexedQuery.java +++ b/java/com/google/gerrit/index/query/IndexedQuery.java @@ -87,6 +87,12 @@ public class IndexedQuery<I, T> extends Predicate<T> implements DataSource<T>, P } @Override + public ResultSet<T> restart(int start) { + opts = opts.withStart(start); + return search(); + } + + @Override public ResultSet<T> restart(int start, int pageSize) { opts = opts.withStart(start).withPageSize(pageSize); return search(); diff --git a/java/com/google/gerrit/index/query/Paginated.java b/java/com/google/gerrit/index/query/Paginated.java index 552199093b..1065019c12 100644 --- a/java/com/google/gerrit/index/query/Paginated.java +++ b/java/com/google/gerrit/index/query/Paginated.java @@ -19,6 +19,8 @@ import com.google.gerrit.index.QueryOptions; public interface Paginated<T> { QueryOptions getOptions(); + ResultSet<T> restart(int start); + ResultSet<T> restart(int start, int pageSize); ResultSet<T> restart(Object searchAfter, int pageSize); diff --git a/java/com/google/gerrit/index/query/PaginatingSource.java b/java/com/google/gerrit/index/query/PaginatingSource.java index 4eceb319e7..8847005629 100644 --- a/java/com/google/gerrit/index/query/PaginatingSource.java +++ b/java/com/google/gerrit/index/query/PaginatingSource.java @@ -55,47 +55,66 @@ public class PaginatingSource<T> implements DataSource<T> { List<T> r = new ArrayList<>(); T last = null; int pageResultSize = 0; + boolean skipped = false; for (T data : buffer(resultSet)) { if (!isMatchable() || match(data)) { r.add(data); + } else { + skipped = true; } last = data; pageResultSize++; } - 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. - // - @SuppressWarnings("unchecked") - Paginated<T> p = (Paginated<T>) source; - QueryOptions opts = p.getOptions(); - final int limit = opts.limit(); - int pageSize = opts.pageSize(); - int pageSizeMultiplier = opts.pageSizeMultiplier(); - Object searchAfter = resultSet.searchAfter(); - int nextStart = pageResultSize; - while (pageResultSize == pageSize && r.size() <= limit) { // get 1 more than the limit - pageSize = getNextPageSize(pageSize, pageSizeMultiplier); - ResultSet<T> next = - indexConfig.paginationType().equals(PaginationType.SEARCH_AFTER) - ? p.restart(searchAfter, pageSize) - : p.restart(nextStart, pageSize); - pageResultSize = 0; - for (T data : buffer(next)) { - if (match(data)) { - r.add(data); + 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 + if (!indexConfig.paginationType().equals(PaginationType.NONE)) { + // Restart source and continue if we have not filled the + // full limit the caller wants. + // + @SuppressWarnings("unchecked") + Paginated<T> p = (Paginated<T>) source; + QueryOptions opts = p.getOptions(); + final int limit = opts.limit(); + int pageSize = opts.pageSize(); + int pageSizeMultiplier = opts.pageSizeMultiplier(); + Object searchAfter = resultSet.searchAfter(); + int nextStart = pageResultSize; + while (pageResultSize == pageSize && r.size() <= limit) { // get 1 more than the limit + pageSize = getNextPageSize(pageSize, pageSizeMultiplier); + ResultSet<T> next = + indexConfig.paginationType().equals(PaginationType.SEARCH_AFTER) + ? p.restart(searchAfter, pageSize) + : p.restart(nextStart, pageSize); + pageResultSize = 0; + for (T data : buffer(next)) { + if (match(data)) { + r.add(data); + } + pageResultSize++; + } + nextStart += pageResultSize; + searchAfter = next.searchAfter(); + } + } else { + @SuppressWarnings("unchecked") + Paginated<T> p = (Paginated<T>) source; + while (skipped && r.size() < p.getOptions().limit() + start) { + skipped = false; + ResultSet<T> next = p.restart(pageResultSize); + + for (T data : buffer(next)) { + if (match(data)) { + r.add(data); + } else { + skipped = true; + } + pageResultSize++; } - pageResultSize++; } - nextStart += pageResultSize; - searchAfter = next.searchAfter(); } } diff --git a/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/java/com/google/gerrit/lucene/LuceneChangeIndex.java index ca73fdcff4..3fc4de2038 100644 --- a/java/com/google/gerrit/lucene/LuceneChangeIndex.java +++ b/java/com/google/gerrit/lucene/LuceneChangeIndex.java @@ -464,11 +464,11 @@ public class LuceneChangeIndex implements ChangeIndex { IndexSearcher[] searchers = new IndexSearcher[indexes.size()]; Map<ChangeSubIndex, ScoreDoc> searchAfterBySubIndex = new HashMap<>(); try { - int realPageSize = opts.start() + opts.pageSize(); - if (Integer.MAX_VALUE - opts.pageSize() < opts.start()) { - realPageSize = Integer.MAX_VALUE; + int pageLimit = AbstractLuceneIndex.getLimitBasedOnPaginationType(opts, opts.pageSize()); + int queryLimit = opts.start() + pageLimit; + if (Integer.MAX_VALUE - pageLimit < opts.start()) { + queryLimit = 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++) { @@ -476,7 +476,7 @@ public class LuceneChangeIndex implements ChangeIndex { searchers[i] = subIndex.acquire(); if (isSearchAfterPagination) { ScoreDoc searchAfter = getSearchAfter(subIndex); - int maxRemainingHits = realPageSize - searchAfterHitsCount; + int maxRemainingHits = queryLimit - searchAfterHitsCount; if (maxRemainingHits > 0) { TopFieldDocs subIndexHits = searchers[i].searchAfter( diff --git a/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java index 4996ff95d7..603564c59e 100644 --- a/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java +++ b/javatests/com/google/gerrit/server/query/change/LuceneQueryChangesTest.java @@ -33,7 +33,6 @@ import com.google.inject.Injector; import org.eclipse.jgit.junit.TestRepository; import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.revwalk.RevCommit; -import org.junit.Ignore; import org.junit.Test; public abstract class LuceneQueryChangesTest extends AbstractQueryChangesTest { @@ -102,7 +101,7 @@ public abstract class LuceneQueryChangesTest extends AbstractQueryChangesTest { assertQuery(newQuery("project:repo").withNoLimit(), expected); } - @Ignore + @Test public void skipChangesNotVisible() throws Exception { // create 1 new change on a repo TestRepository<Repo> repo = createProject("repo"); |