sort based on the textScore for the text search #235
Conversation
| @@ -496,7 +497,13 @@ private Query<PersistentUserSet> buildUserConditionsQuery(UserSetQuery query) { | |||
| private void buildSortCriteria(UserSetQuery query, Query<PersistentUserSet> mongoQuery) { | |||
| for (String sortField : query.getSortCriteria()) { | |||
| if (!sortField.contains(" ")) { | |||
| mongoQuery.order(Sort.ascending(sortField)); | |||
| //check the text search score | |||
There was a problem hiding this comment.
this is the case when no order is provided through the query.
| } | ||
| else { | ||
| mongoQuery.order(Sort.ascending(sortField)); | ||
| } |
There was a problem hiding this comment.
the sort by text score needs to be added when the sort order is provided as well. E.g. sort=score asc/desc must be supported as well in the following else block
| @@ -299,7 +299,8 @@ public void searchSetByTextQueryDefault() throws Exception { | |||
| .param(CommonApiConstants.QUERY_PARAM_PROFILE, LdProfiles.STANDARD.name()) | |||
| .queryParam(CommonApiConstants.PARAM_WSKEY, API_KEY) | |||
| .queryParam(CommonApiConstants.QUERY_PARAM_QUERY, query) | |||
| .queryParam(CommonApiConstants.QUERY_PARAM_PAGE_SIZE, PAGE_SIZE)) | |||
| .queryParam(CommonApiConstants.QUERY_PARAM_PAGE_SIZE, PAGE_SIZE) | |||
| .queryParam(CommonApiConstants.QUERY_PARAM_SORT, WebUserSetFields.TEXT_SCORE_SORT)) | |||
There was a problem hiding this comment.
the sorting by score with asc and desc order needs to be tested in own test and 2 set are needed to check that the sorting+ordering works correctly
There was a problem hiding this comment.
The score sort supports only the desc order per default, and cannot be asc.
There was a problem hiding this comment.
even so, the behaviour needs to be tested, eventually throw and exception of "sort=test asc" is used in the params
There was a problem hiding this comment.
the sort is a user input and the validation of the user input needs to be applied, Consiquently the api behaviour needs to be covered by integration tests as well
| searchQuery.setPageSize(pageSize); | ||
| searchQuery.setPageNr(page); | ||
|
|
||
| return searchQuery; | ||
| } | ||
|
|
||
| private void addSortCriterion(Map<String, String> searchCriteria, UserSetQuery searchQuery, String sort) throws ParamValidationException { | ||
| if (sort!=null && sort.contains(WebUserSetFields.TEXT_SCORE_SORT) && !searchCriteria.containsKey(WebUserSetFields.TEXT)) { | ||
| throw new ParamValidationException(I18nConstants.INVALID_PARAM_VALUE, I18nConstants.INVALID_PARAM_VALUE, |
There was a problem hiding this comment.
this verification needs to be made in the parameter verification phase in the controller
There was a problem hiding this comment.
Since we now throw an exception if "score asc" order is in the input, I keep the processing of the exceptions here. Otherwise the processing would need to be added to the parseCriterion method, which would require to call the toArray method twice, etc. Please see also that the exceptions are also handled in other add* methods in the buildSearchQuery method. It poses a certain overhead, but a very minimalistic one, I would say.
| @@ -299,7 +299,8 @@ public void searchSetByTextQueryDefault() throws Exception { | |||
| .param(CommonApiConstants.QUERY_PARAM_PROFILE, LdProfiles.STANDARD.name()) | |||
| .queryParam(CommonApiConstants.PARAM_WSKEY, API_KEY) | |||
| .queryParam(CommonApiConstants.QUERY_PARAM_QUERY, query) | |||
| .queryParam(CommonApiConstants.QUERY_PARAM_PAGE_SIZE, PAGE_SIZE)) | |||
| .queryParam(CommonApiConstants.QUERY_PARAM_PAGE_SIZE, PAGE_SIZE) | |||
| .queryParam(CommonApiConstants.QUERY_PARAM_SORT, WebUserSetFields.TEXT_SCORE_SORT)) | |||
There was a problem hiding this comment.
even so, the behaviour needs to be tested, eventually throw and exception of "sort=test asc" is used in the params
| @@ -299,7 +299,8 @@ public void searchSetByTextQueryDefault() throws Exception { | |||
| .param(CommonApiConstants.QUERY_PARAM_PROFILE, LdProfiles.STANDARD.name()) | |||
| .queryParam(CommonApiConstants.PARAM_WSKEY, API_KEY) | |||
| .queryParam(CommonApiConstants.QUERY_PARAM_QUERY, query) | |||
| .queryParam(CommonApiConstants.QUERY_PARAM_PAGE_SIZE, PAGE_SIZE)) | |||
| .queryParam(CommonApiConstants.QUERY_PARAM_PAGE_SIZE, PAGE_SIZE) | |||
| .queryParam(CommonApiConstants.QUERY_PARAM_SORT, WebUserSetFields.TEXT_SCORE_SORT)) | |||
There was a problem hiding this comment.
the sort is a user input and the validation of the user input needs to be applied, Consiquently the api behaviour needs to be covered by integration tests as well
| searchQuery.setPageSize(pageSize); | ||
| searchQuery.setPageNr(page); | ||
|
|
||
| return searchQuery; | ||
| } | ||
|
|
||
| private void addSortCriterion(Map<String, String> searchCriteria, UserSetQuery searchQuery, String sort) throws ParamValidationException { | ||
| //validate sorting based on score (can be only in the descending order) | ||
| if(sort!=null && sort.contains(WebUserSetFields.TEXT_SCORE_SORT) && !searchCriteria.containsKey(WebUserSetFields.TEXT)) { |
There was a problem hiding this comment.
the parameter validation including the value provided int the sort is the first step to be performed after the authorization. Please move the validation for the score parameter to the existing method for param validation in the controller method
added code comment
No description provided.