snoopdave commented on code in PR #226:
URL: https://github.com/apache/roller/pull/226#discussion_r4419543544
##########
app/src/main/java/org/apache/roller/weblogger/business/search/lucene/LuceneIndexManager.java:
##########
@@ -432,44 +432,56 @@ static SearchResultList convertHitsToEntryList(
List<WeblogEntryWrapper> results = new ArrayList<>();
- // determine offset
- int offset = pageNum * entryCount;
- if (offset >= hits.length) {
- offset = 0;
- }
-
- // determine limit
- int limit = entryCount;
- if (offset + limit > hits.length) {
- limit = hits.length - offset;
- }
-
try {
- Set<String> categories = new TreeSet<>();
- TreeSet<String> categorySet = new TreeSet<>();
Weblogger roller = WebloggerFactory.getWeblogger();
WeblogEntryManager weblogMgr = roller.getWeblogEntryManager();
- WeblogEntry entry;
- Document doc;
- String handle;
+ // The index can lag behind the entries, so page through the hits
+ // whose entry is published now (and not a future post). Only
+ // those count toward the offset and limit. Stop once the requested
+ // page is filled; if the hits run out first, an out-of-range page
+ // falls back to the first page below.
+ List<WeblogEntry> visible = new ArrayList<>();
+ List<Document> visibleDocs = new ArrayList<>();
Timestamp now = new Timestamp(new Date().getTime());
- for (int i = offset; i < offset + limit; i++) {
- doc = search.getSearcher().doc(hits[i].doc);
- handle =
doc.getField(FieldConstants.WEBSITE_HANDLE).stringValue();
- entry =
weblogMgr.getWeblogEntry(doc.getField(FieldConstants.ID).stringValue());
+ long wanted = (long) (pageNum + 1) * entryCount;
Review Comment:
Hits are capped at 500 (`SearchOperation.docLimit`). For an out-of-range
page this loads an entry for every hit, up to 500 database reads, before it
falls back to page 0. The old code checked `offset >= hits.length` first. Can
we reset to page 0 before the loop when `pageNum * entryCount >= hits.length`?
Crawlers do request high page numbers.
##########
app/src/main/java/org/apache/roller/weblogger/webservices/atomprotocol/EntryCollection.java:
##########
@@ -267,6 +267,8 @@ public void putEntry(AtomRequest areq, Entry entry) throws
AtomException {
CacheManager.invalidate(rollerEntry.getWebsite());
if (rollerEntry.isPublished()) {
roller.getIndexManager().addEntryReIndexOperation(rollerEntry);
+ } else {
+
roller.getIndexManager().removeEntryIndexOperation(rollerEntry);
Review Comment:
Each caller decides add or remove for itself: `EntryEdit` gets it right,
`EntryRemove` uses a set-to-draft trick, and AtomPub had this bug and still has
it on delete. Could we add one `IndexManager` method, for example
`updateEntryIndex(entry)`, that re-indexes if published and removes otherwise,
and use it everywhere? That removes this class of bug instead of fixing one
caller.
##########
app/src/main/java/org/apache/roller/weblogger/business/search/lucene/LuceneIndexManager.java:
##########
@@ -432,44 +432,56 @@ static SearchResultList convertHitsToEntryList(
List<WeblogEntryWrapper> results = new ArrayList<>();
- // determine offset
- int offset = pageNum * entryCount;
- if (offset >= hits.length) {
- offset = 0;
- }
-
- // determine limit
- int limit = entryCount;
- if (offset + limit > hits.length) {
- limit = hits.length - offset;
- }
-
try {
- Set<String> categories = new TreeSet<>();
- TreeSet<String> categorySet = new TreeSet<>();
Weblogger roller = WebloggerFactory.getWeblogger();
WeblogEntryManager weblogMgr = roller.getWeblogEntryManager();
- WeblogEntry entry;
- Document doc;
- String handle;
+ // The index can lag behind the entries, so page through the hits
+ // whose entry is published now (and not a future post). Only
+ // those count toward the offset and limit. Stop once the requested
+ // page is filled; if the hits run out first, an out-of-range page
+ // falls back to the first page below.
+ List<WeblogEntry> visible = new ArrayList<>();
+ List<Document> visibleDocs = new ArrayList<>();
Review Comment:
Minor: `visible` and `visibleDocs` must stay aligned by index. The second
loop needs only the handle and category from each doc, so one list of a small
holder class (entry, handle, category) would be clearer.
--
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]
