-
Notifications
You must be signed in to change notification settings - Fork 788
Add sort arugment for the search API #4865
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA).
To sign the OCA, please create an Oracle account and sign the OCA in Oracle's Contributor Agreement Application. When signing the OCA, please provide your GitHub username. After signing the OCA and getting an OCA approval from Oracle, this PR will be automatically updated. If you are an Oracle employee, please make sure that you are a member of the main Oracle GitHub organization, and your membership in this organization is public. |
7ec47d8
to
58d1dfa
Compare
@QueryParam("projects") final List<String> projects, | ||
@QueryParam("maxresults") // Akin to QueryParameters.COUNT_PARAM | ||
@DefaultValue(MAX_RESULTS + "") final int maxResults, | ||
@QueryParam(QueryParameters.SORT_PARAM) @DefaultValue("relevancy") final String sort, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use SortOrder.RELEVANCY.toString()
instead of the hard-coded string literal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, the Javadoc in IncomingFilter
needs to be updated for the changed method signature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cannot use SortOrder.RELEVANCY.toString() directly in the @DefaultValue annotation, because Java requires annotation values to be compile-time constants.
The Apirary documentation in the |
searcher.search(query, collector); | ||
totalHits = collector.getTotalHits(); | ||
stat.report(LOGGER, Level.FINEST, "search via SearchEngine done", | ||
"search.latency", new String[]{"category", "engine", | ||
"outcome", totalHits > 0 ? "success" : "empty"}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if such pieces could be refactored to avoid code duplication.
Also, needs a test case. |
hits = collector.topDocs().scoreDocs; | ||
} else { | ||
// Field based sort; use TopFieldCollector | ||
TopFieldCollector fieldCollector = TopFieldCollector.create(luceneSort, hitsPerPage * cachePages, Short.MAX_VALUE); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does this work w.r.t. the private TopScoreDocCollector collector;
on line 135/143 ? In the luceneSort == null
branch the private member is assigned however here local variable is used.
What about the results()
function which uses this member ?
Just a note: this conflicts with the changes done for Lucene 9.x update in PR #4867. It looks like this can be reconciled easily, though. |
I signed the OCA on Friday any idea when it will be approved? |
a33b595
to
cb9fd70
Compare
Signed-off-by: Bernát Gábor <[email protected]>
cb9fd70
to
cfc50a1
Compare
Fixes #4740