AO-22 Add support for Advanced Query Syntax #70
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,10 +24,13 @@ | |
| import org.elasticsearch.index.query.QueryBuilders; | ||
| import org.elasticsearch.search.builder.SearchSourceBuilder; | ||
| import org.elasticsearch.search.sort.SortOrder; | ||
|
|
||
| import org.openmrs.addonindex.domain.AddOnInfoAndVersions; | ||
| import org.openmrs.addonindex.domain.AddOnInfoSummary; | ||
| import org.openmrs.addonindex.domain.AddOnInfoSummaryAndStats; | ||
| import org.openmrs.addonindex.domain.AddOnStatus; | ||
| import org.openmrs.addonindex.domain.AddOnType; | ||
|
|
||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
| import org.springframework.beans.factory.annotation.Autowired; | ||
|
|
@@ -70,12 +73,14 @@ public void setUp() throws IOException { | |
| } else { | ||
| // need to create the index | ||
| logger.info("Creating new ES index: " + AddOnInfoAndVersions.ES_INDEX); | ||
| handleError(client.execute(new CreateIndex.Builder(AddOnInfoAndVersions.ES_INDEX).settings( | ||
| loadResource("elasticsearch/addOnInfoAndVersions-settings.json")).build())); | ||
| } | ||
| logger.info("Updating mappings on ES index"); | ||
| handleError(client.execute(new PutMapping.Builder(AddOnInfoAndVersions.ES_INDEX, | ||
| AddOnInfoAndVersions.ES_TYPE, | ||
| loadResource("elasticsearch/addOnInfoAndVersions-mappings.json")) | ||
| .build())); | ||
| } | ||
|
|
||
| private String loadResource(String name) throws IOException { | ||
|
|
@@ -98,8 +103,25 @@ public void index(AddOnInfoAndVersions infoAndVersions) throws IOException { | |
| } | ||
|
|
||
| @Override | ||
| public Collection<AddOnInfoSummary> search(AddOnType type, String query, String tag, String uid, String moduleId, | ||
| String name, String exclude, AddOnStatus status) throws IOException { | ||
| BoolQueryBuilder boolQB = QueryBuilders.boolQuery(); | ||
| if (name != null) { | ||
| //Exact match on name | ||
| boolQB.filter(QueryBuilders.matchQuery("name.raw", name)); | ||
|
There was a problem hiding this comment. this should be case-insensitive. There was a problem hiding this comment. Yes, all are case-insensitive. Keywords are by default case-sensitive. To make this field case-insensitive is why I added a normalizer to it(this feature is only available starting v5.2 of elasticsearch. So, I guess we should update the ReadME to include this) There was a problem hiding this comment. Please update the README. Is that about also sourcing the -setting.json file, or do we also need to update the deployment scripts to use a later version of elasticsearch? |
||
| } | ||
| if (uid != null) { | ||
| //Exact match on uid. This is different from moduleId | ||
| boolQB.filter(QueryBuilders.matchQuery("uid", uid)); | ||
| } | ||
| if (moduleId != null) { | ||
| //Exact match on moduleID | ||
| boolQB.filter(QueryBuilders.matchQuery("moduleId", moduleId)); | ||
| } | ||
| if (status != null) { | ||
| //Exact match on status | ||
| boolQB.filter(QueryBuilders.matchQuery("status", status)); | ||
| } | ||
| if (type != null) { | ||
| //Exact match on type | ||
| boolQB.filter(QueryBuilders.matchQuery("type", type)); | ||
|
|
@@ -127,6 +149,11 @@ public Collection<AddOnInfoSummary> search(AddOnType type, String query, String | |
| boolQB.should(QueryBuilders.matchPhraseQuery("name", query).slop(2).fuzziness("AUTO")); | ||
| boolQB.minimumNumberShouldMatch(1); | ||
| } | ||
|
|
||
| if (exclude != null) { | ||
| boolQB.mustNot(QueryBuilders.multiMatchQuery(exclude , "name", "tags", "description", | ||
| "moduleId", "status", "_id", "type")); | ||
| } | ||
|
|
||
| BoostingQueryBuilder boostingQB = QueryBuilders.boostingQuery(); | ||
| boostingQB.positive(boolQB); | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| { | ||
|
There was a problem hiding this comment. Please give some more justification of why you're doing this (ideally in a code comment somewhere) |
||
| "analysis": { | ||
| "normalizer": { | ||
| "case_normalizer": { | ||
| "type": "custom", | ||
| "filter": ["lowercase"] | ||
| } | ||
| } | ||
| } | ||
|
|
||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ | |
| import {Component} from "react"; | ||
| import {withRouter} from "react-router"; | ||
| import {Button, Col, Form, FormControl, InputGroup} from "react-bootstrap"; | ||
| import searchQuery from "search-query-parser"; | ||
|
|
||
| class SearchBox extends Component { | ||
|
|
||
|
|
@@ -21,43 +22,57 @@ class SearchBox extends Component { | |
|
|
||
| setQuery(query) { | ||
| this.setState({ | ||
| query: query | ||
| }); | ||
| } | ||
|
|
||
| doSearch() { | ||
| if (this.state.query) { | ||
| let query = this.state.query.toLowerCase(); | ||
| const options = {keywords: ['uid', 'type', 'tag', 'query', 'moduleid', 'status', 'name'], offsets: false}; | ||
|
There was a problem hiding this comment. pull this const outside of the function |
||
| //Basic Regex Matching to fix query inconsistencies | ||
| //Removing all extra spaces i.e. two or more | ||
| query = query.replace(/\s+/g,' ').trim(); | ||
| //Removing all spaces to the left of search keys | ||
| query = query.replace(new RegExp("\\s+:","g"),":"); | ||
| //Removing all spaces to the right of search keys | ||
| query = query.replace(new RegExp(":\\s+","g"),":"); | ||
|
There was a problem hiding this comment. Can you pull all this into a helper function like normalizeQuery()? |
||
| let url = "/search?"; | ||
| // Check if query is an advanced query. We want to use the Parser only if the query is advanced | ||
| if (query.includes(":") || query.includes("-")) { | ||
|
There was a problem hiding this comment. Peeking at https://www.npmjs.com/package/search-query-parser it says
so you should rely on this, and avoid doing this test yourself. (E.g. what if I query for: notAValidKey:foo ?) There was a problem hiding this comment. @djazayeri , although from what's mentioned in the docs, it seems that the parser just returns a string, it actually returns something like query: "user query" . This makes it impossible to differentiate a normal query from an advanced one. That is why I had implemented this test to check the pre-parsed query |
||
| let searchQueryObj = searchQuery.parse(query, options); | ||
| Object.keys(searchQueryObj).forEach(function (key) { | ||
| console.log(key); | ||
|
There was a problem hiding this comment. need to remove this before merging the PR |
||
| if (searchQueryObj[key]){ | ||
| if (key === "text" || key === "query") { | ||
| url += "&q=" + searchQueryObj[key]; | ||
| } | ||
| else if (key === "type"){ | ||
| url += "&type=" + searchQueryObj[key].toUpperCase(); | ||
| } | ||
| else if (key === "exclude"){ | ||
| if (searchQueryObj[key].text){ | ||
| url += "&exclude=" + searchQueryObj[key].text; | ||
| } | ||
| } | ||
| else if (key === "status"){ | ||
|
There was a problem hiding this comment. you're handling "type" and "status" in the same way, can you combine these branches? |
||
| url += "&status=" + searchQueryObj[key].toUpperCase(); | ||
| } | ||
| else { | ||
| url += "&" + key + "=" + searchQueryObj[key]; | ||
| } | ||
| } | ||
| }); | ||
| } | ||
| else { | ||
| url += "&q=" + query; | ||
| } | ||
|
|
||
| this.props.router.push(url); | ||
| } | ||
| } | ||
|
|
||
| render() { | ||
| return ( | ||
| <div className="row pushdown"> | ||
| <Form onSubmit={(evt) => { | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
In the big picture it would be nice to avoid changing method signature and adding these parameters all over the place, by just doing the query parsing entirely within the ElasticSearchIndex class.
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.
Yes, that's true, I have currently asked around on forums. Let's hope a java based parser that's similar to the one we're currently using shows up
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.
Conclusion: we didn't find a Java library for this, so we'll proceed doing it in the front end in JS.