diff --git a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/search/SearchAndFilterViewController.java b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/search/SearchAndFilterViewController.java index acddb6b90c..7ae51f6116 100644 --- a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/search/SearchAndFilterViewController.java +++ b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/search/SearchAndFilterViewController.java @@ -230,9 +230,7 @@ public void initialize(URL url, ResourceBundle resourceBundle) { if (e.getCode() == KeyCode.ENTER) { LOGGER.log(Level.INFO, "ENTER has been pressed in uniqueIdTextField"); LOGGER.log(Level.INFO, "uniqueIdProperty: " + uniqueIdProperty.getValueSafe()); - if (uniqueIdProperty.isEmpty().get()) { - LOGGER.log(Level.INFO, "uniqueIdString: is empty"); - } else { + if (!uniqueIdProperty.isEmpty().get()) { searchResultTableViewController.uniqueIdSearch(uniqueIdProperty.getValueSafe()); } } @@ -493,13 +491,13 @@ private void updateParametersAndSearch() { */ private String buildQueryString() { Map map = new HashMap<>(); - if (nodeNameProperty.get() != null && !nodeNameProperty.get().isEmpty()) { + if (nodeNameProperty.get() != null && !nodeNameProperty.get().trim().isEmpty()) { map.put(Keys.NAME.getName(), nodeNameProperty.get()); } if (userNameProperty.get() != null && !userNameProperty.get().isEmpty()) { map.put(Keys.USER.getName(), userNameProperty.get()); } - if (descProperty.get() != null && !descProperty.get().isEmpty()) { + if (descProperty.get() != null && !descProperty.get().trim().isEmpty()) { map.put(Keys.DESC.getName(), descProperty.get()); } if (tagsProperty.get() != null && !tagsProperty.get().isEmpty()) { diff --git a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/search/SearchResultTableViewController.java b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/search/SearchResultTableViewController.java index fad2ebe32c..acfc927383 100644 --- a/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/search/SearchResultTableViewController.java +++ b/app/save-and-restore/app/src/main/java/org/phoebus/applications/saveandrestore/ui/search/SearchResultTableViewController.java @@ -330,6 +330,7 @@ public void search(final String query) { queryString = query; Map searchParams = SearchQueryUtil.parseHumanReadableQueryString(queryString); + LOGGER.log(Level.INFO, "search() searchParams: = " + searchParams); searchParams.put(SearchQueryUtil.Keys.FROM.getName(), Integer.toString(pagination.getCurrentPageIndex() * pageSizeProperty.get())); searchParams.put(SearchQueryUtil.Keys.SIZE.getName(), Integer.toString(pageSizeProperty.get())); @@ -368,7 +369,6 @@ void uniqueIdSearch(final String uniqueIdString) { try { /* Search with the uniqueID */ Node uniqueIdNode = SaveAndRestoreService.getInstance().getNode(uniqueIdString); - LOGGER.log(Level.INFO, "uniqueIDNode: " + uniqueIdNode); /* Check that there are results, then fill table - should be at most one result */ if (uniqueIdNode != null) { diff --git a/services/save-and-restore/src/main/java/org/phoebus/service/saveandrestore/search/SearchUtil.java b/services/save-and-restore/src/main/java/org/phoebus/service/saveandrestore/search/SearchUtil.java index ff6ee9e4e3..7c7d4f2785 100644 --- a/services/save-and-restore/src/main/java/org/phoebus/service/saveandrestore/search/SearchUtil.java +++ b/services/save-and-restore/src/main/java/org/phoebus/service/saveandrestore/search/SearchUtil.java @@ -7,8 +7,10 @@ import co.elastic.clients.elasticsearch._types.query_dsl.BoolQuery.Builder; import co.elastic.clients.elasticsearch.core.SearchRequest; import org.phoebus.applications.saveandrestore.model.Tag; +import org.phoebus.applications.saveandrestore.model.search.SearchQueryUtil; import org.springframework.beans.factory.annotation.Value; import org.springframework.http.HttpStatus; +import org.springframework.ldap.core.support.AbstractContextSource; import org.springframework.util.MultiValueMap; import org.springframework.web.server.ResponseStatusException; @@ -20,6 +22,9 @@ import java.util.Map.Entry; import java.util.stream.Collectors; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + /** * A utility class for creating a search query for log entries based on time, * logbooks, tags, properties, description, etc. @@ -45,6 +50,8 @@ public class SearchUtil { @Value("${elasticsearch.result.size.search.max:1000}") private int maxSearchSize; + private static final Logger LOG = LoggerFactory.getLogger(SearchUtil.class); + /** * @param searchParameters - the various search parameters * @return A {@link SearchRequest} based on the provided search parameters @@ -53,6 +60,7 @@ public SearchRequest buildSearchRequest(MultiValueMap searchPara Builder boolQueryBuilder = new Builder(); boolean fuzzySearch = false; List descriptionTerms = new ArrayList<>(); + List descriptionPhraseTerms = new ArrayList<>(); List nodeNameTerms = new ArrayList<>(); List nodeNamePhraseTerms = new ArrayList<>(); List nodeTypeTerms = new ArrayList<>(); @@ -63,6 +71,9 @@ public SearchRequest buildSearchRequest(MultiValueMap searchPara int searchResultSize = defaultSearchSize; int from = 0; + LOG.info("buildSearchRequest() called"); + LOG.info(" searchParameters: " + searchParameters); + for (Entry> parameter : searchParameters.entrySet()) { switch (parameter.getKey().strip().toLowerCase()) { case "uniqueid": @@ -72,29 +83,41 @@ public SearchRequest buildSearchRequest(MultiValueMap searchPara } } break; + // Search for node name. List of names cannot be split on space char as it is allowed in a node name. case "name": for (String value : parameter.getValue()) { - for (String pattern : value.split("[|,;]")) { + for (String pattern : getSearchTerms(value)) { String term = pattern.trim().toLowerCase(); + // Quoted strings will be mapped to a phrase query if(term.startsWith("\"") && term.endsWith("\"")){ nodeNamePhraseTerms.add(term.substring(1, term.length() - 1)); } else{ - nodeNameTerms.add(term); + // add wildcards inorder to search for sub-strings + nodeNameTerms.add("*" + term + "*"); } } } break; + // Search in description/comment - case "description": case "desc": + case "description": for (String value : parameter.getValue()) { - for (String pattern : value.split("[|,;]")) { - descriptionTerms.add(pattern.trim()); + for (String pattern : getSearchTerms(value)) { + String term = pattern.trim().toLowerCase(); + // Quoted strings will be mapped to a phrase query + if (term.startsWith("\"") && term.endsWith("\"")) { + descriptionPhraseTerms.add(term.substring(1, term.length() - 1)); + } else { + // add wildcards inorder to search for sub-strings + descriptionTerms.add("*" + term + "*"); + } } } break; + // Search for node type. case "type": for (String value : parameter.getValue()) { @@ -212,30 +235,28 @@ public SearchRequest buildSearchRequest(MultiValueMap searchPara } } - // Add the description query + // Add the description query. Multiple search terms will be AND:ed. if (!descriptionTerms.isEmpty()) { - DisMaxQuery.Builder descQuery = new DisMaxQuery.Builder(); - List descQueries = new ArrayList<>(); - if (fuzzySearch) { - descriptionTerms.forEach(searchTerm -> { - Query fuzzyQuery = FuzzyQuery.of(f -> f.field("node.description").value(searchTerm))._toQuery(); - NestedQuery nestedQuery = - NestedQuery.of(n1 -> n1.path("node") - .query(fuzzyQuery)); - descQueries.add(nestedQuery._toQuery()); - }); - } else { - descriptionTerms.forEach(searchTerm -> { - Query wildcardQuery = - WildcardQuery.of(w -> w.field("node.description").value(searchTerm))._toQuery(); - NestedQuery nestedQuery = - NestedQuery.of(n1 -> n1.path("node") - .query(wildcardQuery)); - descQueries.add(nestedQuery._toQuery()); - }); + for (String searchTerm : descriptionTerms) { + NestedQuery innerNestedQuery; + if (fuzzySearch) { + FuzzyQuery matchQuery = FuzzyQuery.of(m -> m.field("node.description").value(searchTerm)); + innerNestedQuery = NestedQuery.of(n -> n.path("node").query(matchQuery._toQuery())); + } else { + WildcardQuery matchQuery = WildcardQuery.of(m -> m.field("node.description").value(searchTerm)); + innerNestedQuery = NestedQuery.of(n -> n.path("node").query(matchQuery._toQuery())); + } + boolQueryBuilder.must(innerNestedQuery._toQuery()); + } + } + + // Add phrase queries for the description key. Multiple search terms will be AND:ed. + if (!descriptionPhraseTerms.isEmpty()) { + for (String searchTerm : descriptionPhraseTerms) { + MatchPhraseQuery matchQuery = MatchPhraseQuery.of(m -> m.field("node.description").query(searchTerm)); + NestedQuery innerNestedQuery = NestedQuery.of(n -> n.path("node").query(matchQuery._toQuery())); + boolQueryBuilder.must(innerNestedQuery._toQuery()); } - descQuery.queries(descQueries); - boolQueryBuilder.must(descQuery.build()._toQuery()); } // Add uniqueId query @@ -243,40 +264,28 @@ public SearchRequest buildSearchRequest(MultiValueMap searchPara boolQueryBuilder.must(IdsQuery.of(id -> id.values(uniqueIdTerms))._toQuery()); } - // Add the name query + // Add the description query. Multiple search terms will be AND:ed. if (!nodeNameTerms.isEmpty()) { - DisMaxQuery.Builder nodeNameQuery = new DisMaxQuery.Builder(); - List nodeNameQueries = new ArrayList<>(); - if (fuzzySearch) { - nodeNameTerms.forEach(searchTerm -> { - NestedQuery innerNestedQuery; + for (String searchTerm : nodeNameTerms) { + NestedQuery innerNestedQuery; + if (fuzzySearch) { FuzzyQuery matchQuery = FuzzyQuery.of(m -> m.field("node.name").value(searchTerm)); - innerNestedQuery = NestedQuery.of(n1 -> n1.path("node").query(matchQuery._toQuery())); - nodeNameQueries.add(innerNestedQuery._toQuery()); - }); - } else { - nodeNameTerms.forEach(searchTerm -> { - NestedQuery innerNestedQuery; + innerNestedQuery = NestedQuery.of(n -> n.path("node").query(matchQuery._toQuery())); + } else { WildcardQuery matchQuery = WildcardQuery.of(m -> m.field("node.name").value(searchTerm)); - innerNestedQuery = NestedQuery.of(n1 -> n1.path("node").query(matchQuery._toQuery())); - nodeNameQueries.add(innerNestedQuery._toQuery()); - }); + innerNestedQuery = NestedQuery.of(n -> n.path("node").query(matchQuery._toQuery())); + } + boolQueryBuilder.must(innerNestedQuery._toQuery()); } - nodeNameQuery.queries(nodeNameQueries); - boolQueryBuilder.must(nodeNameQuery.build()._toQuery()); } - if(!nodeNamePhraseTerms.isEmpty()){ - DisMaxQuery.Builder nodeNamePhraseQueryBuilder = new DisMaxQuery.Builder(); - List nestedQueries = new ArrayList<>(); - nodeNamePhraseTerms.forEach(phraseSearchTerm -> { - NestedQuery innerNestedQuery; - MatchPhraseQuery matchPhraseQuery = MatchPhraseQuery.of(m -> m.field("node.name").query(phraseSearchTerm)); - innerNestedQuery = NestedQuery.of(n -> n.path("node").query(matchPhraseQuery._toQuery())); - nestedQueries.add(innerNestedQuery); - }); - nodeNamePhraseQueryBuilder.queries(nestedQueries.stream().map(QueryVariant::_toQuery).collect(Collectors.toList())); - boolQueryBuilder.must(nodeNamePhraseQueryBuilder.build()._toQuery()); + // Add phrase queries for the nodeName key. Multiple search terms will be AND:ed. + if (!nodeNamePhraseTerms.isEmpty()) { + for (String searchTerm : nodeNamePhraseTerms) { + MatchPhraseQuery matchQuery = MatchPhraseQuery.of(m -> m.field("node.name").query(searchTerm)); + NestedQuery innerNestedQuery = NestedQuery.of(n -> n.path("node").query(matchQuery._toQuery())); + boolQueryBuilder.must(innerNestedQuery._toQuery()); + } } // Add node type query. Fuzzy search not needed as node types are well-defined and limited in number. @@ -337,4 +346,43 @@ public SearchRequest buildSearchRequestForPvs(List pvNames) { .size(Math.min(searchResultSize, maxSearchSize)) .from(0)); } + + /** + * Parses a search query terms string into a string array. In particular, + * quoted search terms must be maintained even if they contain the + * separator chars used to tokenize the terms. + * + * @param searchQueryTerms String as specified by client + * @return A {@link List} of search terms, some of which may be + * quoted. Is void of any zero-length strings. + */ + public List getSearchTerms(String searchQueryTerms) { + // Count double quote chars. Odd number of quote chars + // is not supported -> throw exception + long quoteCount = searchQueryTerms.chars().filter(c -> c == '\"').count(); + if (quoteCount == 0) { + return Arrays.stream(searchQueryTerms.split("[\\|,;\\s+]")).filter(t -> t.length() > 0).collect(Collectors.toList()); + } + if (quoteCount % 2 == 1) { + throw new IllegalArgumentException("Unbalanced quotes in search query"); + } + // If we come this far then at least one quoted term is + // contained in user input + List terms = new ArrayList<>(); + int nextStartIndex = searchQueryTerms.indexOf('\"'); + while (nextStartIndex >= 0) { + int endIndex = searchQueryTerms.indexOf('\"', nextStartIndex + 1); + String quotedTerm = searchQueryTerms.substring(nextStartIndex, endIndex + 1); + terms.add(quotedTerm); + // Remove the quoted term from user input + searchQueryTerms = searchQueryTerms.replace(quotedTerm, ""); + // Check next occurrence + nextStartIndex = searchQueryTerms.indexOf('\"'); + } + // Add remaining terms... + List remaining = Arrays.asList(searchQueryTerms.split("[\\|,;\\s+]")); + //...but remove empty strings, which are "leftovers" when quoted terms are removed + terms.addAll(remaining.stream().filter(t -> t.length() > 0).collect(Collectors.toList())); + return terms; + } }