-
Notifications
You must be signed in to change notification settings - Fork 37
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
AO-22 Add support for Advanced Query Syntax #70
base: master
Are you sure you want to change the base?
Changes from all commits
33accff
a381e4a
03ead9f
6e88bf2
3102de9
e9ec7fe
0706d4c
fb9d5b6
efdcda1
613134d
c289432
d226200
3259cd0
ca0ca4d
00f7783
bdfcd55
f24fe19
0a5cca6
a6904ca
0804487
f635bc5
a71f7f7
c6cc91c
80a93b4
1a30516
d8a3c47
4f30974
b4c6a57
828ec46
51755a1
069ed9f
62725a5
bf095a1
51e8adb
d41064c
f3cc839
bff4db5
418f919
db524ac
3b2ef2a
534f175
514b05f
df696ba
88cb630
0e39fa3
7ec8a3d
f5fae33
45506f9
77e0a78
f250080
05d4ab8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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,16 @@ 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).build())); | ||
//By default, the keyword field is case sensitive which is not what we want | ||
//In this settings file, we create a custom normalizer to make the keyword field case-insensitive | ||
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())); | ||
loadResource("elasticsearch/addOnInfoAndVersions-mappings.json")) | ||
.build())); | ||
} | ||
|
||
private String loadResource(String name) throws IOException { | ||
|
@@ -98,8 +105,25 @@ public void index(AddOnInfoAndVersions infoAndVersions) throws IOException { | |
} | ||
|
||
@Override | ||
public Collection<AddOnInfoSummary> search(AddOnType type, String query, String tag) throws IOException { | ||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should be case-insensitive. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.keyword", 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 +151,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); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please give some more justification of why you're doing this (ideally in a code comment somewhere) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Resolved |
||
"analysis": { | ||
"normalizer": { | ||
"case_normalizer": { | ||
"type": "custom", | ||
"filter": ["lowercase"] | ||
} | ||
} | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,11 @@ import {Component} from "react"; | |
import fetch from "isomorphic-fetch"; | ||
import SearchBox from "../component/SearchBox"; | ||
import AddOnList from "../component/AddOnList"; | ||
import searchQuery from "search-query-parser"; | ||
|
||
//Configuration for the Search Query Parser | ||
const options = {keywords: ['uid', 'type', 'tag', 'query', 'moduleid', 'status', 'name'], | ||
tokenize:true ,offsets:false}; | ||
|
||
export default class SearchPage extends Component { | ||
|
||
|
@@ -30,28 +35,56 @@ export default class SearchPage extends Component { | |
} | ||
} | ||
|
||
doSearch() { | ||
const query = this.props.location.query.q; | ||
const type = this.props.location.query.type; | ||
const tag = this.props.location.query.tag; | ||
const searchKey = `type:${type ? type : "all"} query:${query ? query : ""} tag:${tag ? tag : ""}`; | ||
normalizeQuery(query) { | ||
//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"),":"); | ||
return query; | ||
} | ||
|
||
doSearch() { | ||
const searchKey = this.props.location.query.q; | ||
let query = searchKey.toLowerCase(); | ||
if (this.state.latestSearch === searchKey) { | ||
return; | ||
} | ||
|
||
this.handleStartSearch(searchKey); | ||
|
||
let url = "/api/v1/addon?"; | ||
if (type) { | ||
url += "type=" + type; | ||
|
||
query = this.normalizeQuery(query); | ||
|
||
// Check if query is an advanced query. We want to use the Parser only if the query is advanced | ||
if (query.includes(":") || query.includes("-")) { | ||
let searchQueryObj = searchQuery.parse(query, options); | ||
Object.keys(searchQueryObj).forEach(function (key) { | ||
if (searchQueryObj[key]) { | ||
if (key === "text" || key === "query") { | ||
key === "text" ? url += "&q=" + searchQueryObj[key].join(" ") : url += "&q=" + searchQueryObj[key]; | ||
} | ||
else if (key === "type" || key === "status") { | ||
url += "&" + key + "=" + searchQueryObj[key].toUpperCase(); | ||
} | ||
else if (key === "exclude") { | ||
if (searchQueryObj[key].text) { | ||
url += "&exclude=" + searchQueryObj[key].text; | ||
} | ||
} | ||
else { | ||
url += "&" + key + "=" + searchQueryObj[key]; | ||
} | ||
} | ||
}); | ||
} | ||
else { | ||
url += "&q=" + query; | ||
} | ||
if (query) { | ||
url += "&q=" + query; | ||
} | ||
if (tag) { | ||
url += "&tag=" + tag; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change implies there should be a change to the URL of the link for searching by tag, right? I don't see it changed in this PR. |
||
} | ||
|
||
fetch(url) | ||
.then(response => { | ||
return response.json(); | ||
|
@@ -77,13 +110,15 @@ export default class SearchPage extends Component { | |
} | ||
|
||
render() { | ||
let query = this.props.location.query.q; | ||
|
||
if (this.state && this.state.error) { | ||
return <div>{this.state.error}</div> | ||
} | ||
else { | ||
return ( | ||
<div> | ||
<SearchBox initialQuery={this.props.location.query.q}/> | ||
<SearchBox initialQuery={query}/> | ||
|
||
{this.state.latestSearch ? | ||
<div>Searching for {this.state.latestSearch}</div> | ||
|
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.