mraible commented on code in PR #249:
URL: https://github.com/apache/roller/pull/249#discussion_r4571273050
##########
app/src/main/java/org/apache/roller/weblogger/business/search/lucene/SearchOperation.java:
##########
@@ -141,7 +140,9 @@ public void doRun() {
.build();
}
- searchresults = searcher.search(query, docLimit, SORTER);
+ // ask for every match, so that results past the first few hundred
+ // can still be paged to
+ searchresults = searcher.search(query, Math.max(1,
reader.maxDoc()), SORTER);
Review Comment:
Agreed on the cost. The search now keeps at most `search.results.max` hits,
5,000 by default and documented in roller.properties, so the queue no longer
scales with the index. I did not take the total from `search(query, n, sort)`:
on Lucene 9.12.3 it reports `GREATER_THAN_OR_EQUAL_TO` once there are more than
1,000 matches and n is smaller than that. The search uses a
`TopFieldCollectorManager` with no total-hits threshold instead, which gives an
exact count in the same pass. The page shows the real count and only paging
depth is capped; the last page inside the cap has no next link.
##########
app/src/main/java/org/apache/roller/weblogger/business/search/lucene/LuceneIndexManager.java:
##########
@@ -513,7 +514,9 @@ static SearchResultList convertHitsToEntryList(
categories = categorySet;
}
- return new SearchResultList(results, categories, limit, offset);
+ boolean more = visible.size() > offset + limit;
+ return new SearchResultList(results, categories, limit, offset,
+ hits.length, more);
Review Comment:
Right, the total is index matches. The Javadoc on `getTotalHits()` now says
so and lists why it can exceed what a visitor can page to: drafts, scheduled
posts, entries deleted since indexing, and the paging cap. I added a test with
a draft in the middle of the hits that asserts the total. I left the message
text alone because "Entries found" is in seven translated bundles; I can reword
it in a follow-up if you want. CHANGES.md and the PR description note the limit.
##########
app/src/test/java/org/apache/roller/weblogger/business/search/lucene/SearchHitPagingTest.java:
##########
@@ -49,11 +51,24 @@ class SearchHitPagingTest {
private static final int HITS = 20;
@Test
- void firstPageLoadsOnlyItsEntries() throws Exception {
+ void firstPageLoadsItsEntriesAndOneMore() throws Exception {
WeblogEntryManager entries = entries(-1);
SearchResultList result = convert(entries, 0, 2);
assertEquals(2, result.getResults().size());
- verify(entries, times(2)).getWeblogEntry(anyString());
+ // the third load is the look ahead that says whether there is a next
page
+ verify(entries, times(3)).getWeblogEntry(anyString());
+ assertTrue(result.hasMore());
+ assertEquals(HITS, result.getTotalHits());
+ }
+
+ @Test
+ void lastPageHasNothingAfterIt() throws Exception {
Review Comment:
Added `SearchResultLimitTest`, which runs `SearchOperation` against an
in-memory index of 1,200 matching entries. It asserts the count is 1,200 and
exact, that page 60 (entries 601–610) and the final page come back, and that
with a small cap paging stops at the cap with no next link while the total
stays 1,200. All four tests fail with `docLimit = 500` restored.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
