snoopdave commented on code in PR #225:
URL: https://github.com/apache/roller/pull/225#discussion_r4419745136
##########
app/src/main/java/org/apache/roller/weblogger/business/search/lucene/IndexOperation.java:
##########
@@ -152,10 +145,6 @@ protected Document getDocument(WeblogEntry data) {
doc.add(new TextField(FieldConstants.C_CONTENT, commentContent,
Field.Store.NO));
- // keyword
- doc.add(new StringField(FieldConstants.C_EMAIL, commentEmail,
- Field.Store.YES));
-
// keyword
doc.add(new StringField(FieldConstants.C_NAME, commentName,
Review Comment:
🤖Claude: fixed by no longer adding `c_name` to the document, nor `username`
(from the review summary). `IndexedFieldsTest` checks a document built from an
entry with an author and a comment holds neither field, nor `c_email`
(eca1553fc).
##########
app/src/main/java/org/apache/roller/weblogger/business/search/lucene/SearchOperation.java:
##########
@@ -229,4 +231,71 @@ public void setLocale(String locale) {
this.locale = locale;
}
+
+ /**
+ * Query parser that accepts explicit field names only for the fields that
+ * searches are meant to cover. Other index fields hold data that is not
+ * shown on the weblog, so a query naming one is a parse error.
+ */
+ static class SearchFieldsQueryParser extends MultiFieldQueryParser {
Review Comment:
🤖Claude: fixed by replacing the seven overrides with one `parse` override:
it parses normally, collects every field through a `QueryVisitor`
(`acceptField` and `consumeTerms`), and refuses any field not in
`SEARCH_FIELDS`. `*:*` needs no special case. All cases in
`SearchFieldsQueryParserTest` pass unchanged (eca1553fc).
##########
CHANGES.md:
##########
@@ -58,6 +58,11 @@
- **Startup fails if the XML-RPC parser cannot be configured.** Roller used to
log an error and continue. It now stops at startup, so check the log if a
custom XML parser is on the classpath.
+- **Search queries can name only the search fields.** A weblog search may
+ name entry content, title or comment text (for example `title:roller`). A
+ query that names another field is treated like a query that cannot be
+ parsed. Comment email addresses are no longer added to the search index;
+ rebuild the index after upgrading to remove those already there.
Review Comment:
🤖Claude: fixed by checking the index at startup: if any segment still has
`username`, `email` or `name` (`FieldConstants.REMOVED_FIELDS`), Roller deletes
the index, creates an empty one and schedules the background rebuild.
`IndexedFieldsTest` covers the check. CHANGES.md now says the rebuild happens
on its own (eca1553fc).
##########
app/src/test/java/org/apache/roller/weblogger/business/search/IndexManagerTest.java:
##########
@@ -97,6 +100,13 @@ public void testBasicOperation() throws Exception {
WebloggerFactory.getWeblogger().getUrlStrategy());
assertEquals(1, result.getResults().size());
+ // index fields that are not search fields cannot be queried
+ String handlePrefix = testWeblog.getHandle().substring(0,
3).toLowerCase() + "*";
+ assertThrows(WebloggerException.class, () -> indexManager.search(
Review Comment:
🤖Claude: not fixed — the test already runs a valid search and asserts one
result just before this check, so a broken search would fail there first.
`SearchFieldsQueryParserTest` covers the refusal itself with `ParseException`.
--
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]
