Better support for elasticsearch characters (#46)

* Support colons in search box & metadata search

* remove logging

* Add support for other operators in search api & search box

* Clean up != behavior, include gte lte in UI

* Clean up regex and parsing for exists and missing queries

* remove space sensitivity to searches

* improve documentation

* Fixed spacing check

* handle colons on contains matches better

* Add numeric detection to mapping

* disable lt gt lte gte

* update help syntax

* Simplify help

* colon warning

Author: max-zilla

app/services/ElasticsearchPlugin.scala

@@ -27,6 +27,7 @@ import models.{Collection, Dataset, ElasticsearchResult, File, Folder, ResourceR
 import play.api.Play.current
 import play.api.libs.json._
 import _root_.util.SearchUtils
+import org.apache.commons.lang.StringUtils
 import org.elasticsearch.action.admin.indices.exists.indices.IndicesExistsRequest
 
 
@@ -48,7 +49,8 @@ class ElasticsearchPlugin(application: Application) extends Plugin {
   val nameOfIndex = play.api.Play.configuration.getString("elasticsearchSettings.indexNamePrefix").getOrElse("clowder")
   val maxResults = play.api.Play.configuration.getInt("elasticsearchSettings.maxResults").getOrElse(240)
 
-  val mustOperators = List("==", "<", ">", ":")
+  // TODO: Removed gt lt gte lte operators until numeric_detection can be enabled on the dynamic mapper
+  val mustOperators = List("==", ":") // "<=", ">=", "<", ">", ":")
   val mustNotOperators = List("!=")
 
 
@@ -130,53 +132,37 @@ class ElasticsearchPlugin(application: Application) extends Plugin {
     accumulatePageResult(queryObj, user, from.getOrElse(0), size.getOrElse(maxResults))
   }
 
-  /** Search using a simple text string, appending parameters from API to string if provided */
+  /**
+   * Search using a simple text string.
+   * The API endpoint supports several parameters like datasetid that are translated and appended to the query first.
+   * @param query
+   * @param resource_type - Restrict to particular resource_type
+   * @param datasetid - Restrict to particular dataset ID (only returns files)
+   * @param collectionid - Restrict to particular collection ID
+   * @param spaceid - Restrict to particular space ID
+   * @param folderid - Restrict to particular folder ID
+   * @param field - Restrict to a specific metadata field (assumes query is the value)
+   * @param tag - Restrict to a particular tag (exact match)
+   * @param from
+   * @param size
+   * @param permitted
+   * @param user
+   * @param index
+   */
   def search(query: String, resource_type: Option[String], datasetid: Option[String], collectionid: Option[String],
              spaceid: Option[String], folderid: Option[String], field: Option[String], tag: Option[String],
              from: Option[Int], size: Option[Int], permitted: List[UUID], user: Option[User],
              index: String = nameOfIndex): ElasticsearchResult = {
 
+    // Convert any parameters from API into the query syntax equivalent so we can parse it all together later
     var expanded_query = query
-
-    // whether to restrict to a particular metadata field, or search all fields (including tags, name, etc.)
-    val mdfield = field match {
-      case Some(k) => expanded_query = " "+k+":\""+expanded_query+"\""
-      case None => {}
-    }
-
-    // Restrict to a particular tag - currently requires exact match
-    tag match {
-      case Some(t) => expanded_query += " tag:"+t
-      case None => {}
-    }
-
-    // Restrict to particular resource_type if requested
-    resource_type match {
-      case Some(restype) => expanded_query += " resource_type:"+restype
-      case None => {}
-    }
-
-    // Restrict to particular dataset ID (only return files)
-    datasetid match {
-      case Some(dsid) => expanded_query += " in:"+dsid+" resource_type:file"
-      case None => {}
-    }
-
-    // Restrict to particular collection ID
-    collectionid match {
-      case Some(cid) => expanded_query += " in:"+cid
-      case None => {}
-    }
-
-    spaceid match {
-      case Some(spid) => expanded_query += " in:"+spid
-      case None => {}
-    }
-
-    folderid match {
-      case Some(fid) => expanded_query += " in:"+fid
-      case None => {}
-    }
+    field.foreach(k => expanded_query = " "+k+":\""+expanded_query+"\"")
+    tag.foreach(t => expanded_query += s" tag:$t")
+    resource_type.foreach(restype => expanded_query += s" resource_type:$restype")
+    datasetid.foreach(dsid => expanded_query += s" in:$dsid resource_type:file")
+    collectionid.foreach(cid => expanded_query += s" in:$cid")
+    spaceid.foreach(spid => expanded_query += s" in:$spid")
+    folderid.foreach(fid => expanded_query += s" in:$fid")
 
     val queryObj = prepareElasticJsonQuery(expanded_query.stripPrefix(" "), permitted, user)
     accumulatePageResult(queryObj, user, from.getOrElse(0), size.getOrElse(maxResults))
@@ -291,7 +277,6 @@ class ElasticsearchPlugin(application: Application) extends Plugin {
     }).toList, response.getHits().getTotalHits())
   }
 
-
   /** Create a new index with preconfigured mappgin */
   def createIndex(index: String = nameOfIndex): Unit = {
     val indexSettings = Settings.settingsBuilder().loadFromSource(jsonBuilder()
@@ -706,10 +691,12 @@ class ElasticsearchPlugin(application: Application) extends Plugin {
 
   /** Return string-encoded JSON object describing field types */
   def getElasticsearchObjectMappings(): String = {
+
     /** The dynamic template will restrict all dynamic metadata fields to be indexed
      * as strings for datatypes besides Objects. In the future, this could
      * be removed, but only once the Search API better supports those data types (e.g. Date).
      */
+    // TODO: Enable "numeric_detection": true alongside date_detection
     """{"clowder_object": {
           |"date_detection": false,
           |"properties": {
@@ -737,17 +724,27 @@ class ElasticsearchPlugin(application: Application) extends Plugin {
 
   /** Create appropriate search object based on operator */
   def parseMustOperators(builder: XContentBuilder, key: String, value: String, operator: String): XContentBuilder = {
-    // TODO: Suppert lte, gte (<=, >=)
     operator match {
       case "==" => builder.startObject().startObject("match_phrase").field(key, value).endObject().endObject()
       case "<" => builder.startObject().startObject("range").startObject(key).field("lt", value).endObject().endObject().endObject()
       case ">" => builder.startObject().startObject("range").startObject(key).field("gt", value).endObject().endObject().endObject()
+      case "<=" => builder.startObject().startObject("range").startObject(key).field("lte", value).endObject().endObject().endObject()
+      case ">=" => builder.startObject().startObject("range").startObject(key).field("gte", value).endObject().endObject().endObject()
       case ":" => {
         if (key == "_all")
           builder.startObject().startObject("regexp").field("_all", wrapRegex(value)).endObject().endObject()
-        else
+        else if (key == "exists") {
+          val cleaned = if (!value.startsWith("metadata.")) "metadata."+value else value
+          builder.startObject().startObject("exists").field("field", cleaned).endObject().endObject()
+        } else if (key == "missing") {
+          val cleaned = if (!value.startsWith("metadata.")) "metadata."+value else value
+          builder.startObject().startObject("bool").startArray("must_not").startObject()
+            .startObject("exists").field("field", cleaned).endObject().endObject().endArray().endObject().endObject()
+        } else {
+          val cleaned = value.replace(":", "\\:") // Colons have special meaning in query_string
           builder.startObject().startObject("query_string").field("default_field", key)
-            .field("query", "\""+value+"\"").endObject().endObject()
+            .field("query", cleaned).endObject().endObject()
+        }
       }
       case _ => {}
     }
@@ -763,7 +760,7 @@ class ElasticsearchPlugin(application: Application) extends Plugin {
     builder
   }
 
-  /**Convert list of search term JsValues into an Elasticsearch-ready JSON query object**/
+  /** Convert list of search term JsValues into an Elasticsearch-ready JSON query object **/
   def prepareElasticJsonQuery(query: List[JsValue], grouping: String): XContentBuilder = {
     /** OPERATORS
       *  :   contains (partial match)
@@ -792,7 +789,7 @@ class ElasticsearchPlugin(application: Application) extends Plugin {
       builder.startArray("should").startObject().startObject("bool")
 
     // 2) populate the MUST/SHOULD portion
-    if (mustList.length > 0) {
+    if (mustList.length > 0 || mustNotList.length > 0) {
       grouping match {
         case "AND" => builder.startArray("must")
         case "OR" => builder.startArray("should")
@@ -803,6 +800,13 @@ class ElasticsearchPlugin(application: Application) extends Plugin {
         val value = (jv \ "field_value").toString.replace("\"", "")
         builder = parseMustOperators(builder, key, value, operator)
       })
+
+      // Also add != fields to MUST EXISTS query so we don't return all documents without those fields too
+      mustNotList.foreach(jv => {
+        val key = (jv \ "field_key").toString.replace("\"","")
+        builder.startObject().startObject("exists").field("field", key).endObject().endObject()
+      })
+
       builder.endArray()
     }
 
@@ -838,27 +842,37 @@ class ElasticsearchPlugin(application: Application) extends Plugin {
     builder
   }
 
-  /**Convert search string into an Elasticsearch-ready JSON query object**/
+  /** Convert search string into an Elasticsearch-ready JSON query object **/
   def prepareElasticJsonQuery(query: String, permitted: List[UUID], user: Option[User]): XContentBuilder = {
-    /** OPERATORS
-      *  ==  equals (exact match)
-      *  !=  not equals (partial matches OK)
-      *  <   less than
-      *  >   greater than
-      **/
 
     // Use regex to split string into a list, preserving quoted phrases as single value
     val matches = ListBuffer[String]()
-    val m = Pattern.compile("([^\"]\\S*|\".+?\")\\s*").matcher(query)
+    val m = Pattern.compile("([^\":= ]+|\".+?\")").matcher(query)
+    //val m = Pattern.compile("([^\":=<> ]+|\".+?\")").matcher(query)
     while (m.find()) {
-      var mat = m.group(1).replace("\"", "").replace("__", " ")
-      if (mat.startsWith(":")) mat = mat.substring(1)
-      if (mat.endsWith(":")) mat = mat.substring(0, mat.length-2)
-      matches += mat
+      var mat = m.group(1).replace("\"", "").replace("__", " ").trim
+      if (mat.length>0) {
+        // Remove operators from terms e.g. <=value becomes value
+        (mustOperators ::: mustNotOperators).foreach(op => {
+          if (mat.startsWith(op)) {
+            // Make sure x<=4 is "x lte 4" not "x lt =4"
+            var foundLonger = false
+            (mustOperators ::: mustNotOperators).foreach(longerop => {
+              if (longerop!=op && longerop.length>op.length && mat.startsWith(longerop)) {
+                mat = mat.substring(longerop.length)
+                foundLonger = true
+              }
+            })
+            if (!foundLonger)
+              mat = mat.substring(op.length)
+          }
+        })
+        matches += mat
+      }
     }
 
     // If a term is specified that isn't in this list, it's assumed to be a metadata field
-    val official_terms = List("name", "creator", "email", "resource_type", "in", "contains", "tag")
+    val official_terms = List("name", "creator", "email", "resource_type", "in", "contains", "tag", "exists", "missing")
 
     // Create list of (key, operator, value) for passing to builder
     val terms = ListBuffer[(String, String, String)]()
@@ -867,36 +881,44 @@ class ElasticsearchPlugin(application: Application) extends Plugin {
     var currval = ""
     matches.foreach(mt => {
       // Check if the current term appears before or after one of the operators, and what operator is
-      var entryType = "value"
-      if (query.contains(mt+":") || query.contains("\""+mt+"\":")) {
-        entryType = "key"
-      }
+      var entryType = "unknown"
+      (mustOperators ::: mustNotOperators).foreach(op => {
+        val reducedSpaces = StringUtils.normalizeSpace(query)
+        if ((reducedSpaces.contains(mt+op) || reducedSpaces.contains("\""+mt+"\""+op) ||
+            reducedSpaces.contains(mt+" "+op) || reducedSpaces.contains("\""+mt+"\" "+op)) && entryType=="unknown") {
+          entryType = "key"
+          curropr = op
+        } else if (reducedSpaces.contains(op+mt) || reducedSpaces.contains(op+"\""+mt+"\"") ||
+            reducedSpaces.contains(op+" "+mt) || reducedSpaces.contains(op+" \""+mt+"\"")) {
+          entryType = "value"
+          curropr = op
+        }
+      })
+      if (entryType=="unknown") entryType = "value"
 
       // Determine if the string was a key or value
       if (entryType == "key") {
         // Do some user-friendly replacement
-        if (mt == "tag")
-          currkey = "tags"
-        else if (mt == "in")
-          currkey = "child_of"
-        else if (mt == "contains")
-          currkey = "parent_of"
-        else if (mt == "creator")
-          currkey = "creator_name"
-        else if (mt == "email")
-          currkey = "creator_email"
-        else if (!official_terms.contains(mt))
-          currkey = "metadata."+mt
+        if (mt == "tag") currkey = "tags"
+        else if (mt == "in") currkey = "child_of"
+        else if (mt == "contains") currkey = "parent_of"
+        else if (mt == "creator") currkey = "creator_name"
+        else if (mt == "email") currkey = "creator_email"
+        else if (!official_terms.contains(mt)) currkey = "metadata."+mt
         else
           currkey = mt
       } else if (entryType == "value") {
-        currval += mt.toLowerCase()
+        if (currkey!="exists" && currkey!="missing")
+          currval = mt.toLowerCase()
+        else currval= mt
         terms += ((currkey, curropr, currval))
         currkey = "_all"
+        curropr = ":"
         currval = ""
       }
     })
 
+    // Now that we have a nicely structured list of (key, operator, value) tuples we can translate to Elastic objects
     var builder = jsonBuilder().startObject().startObject("bool")
 
     // First, populate the MUST portion of Bool query
@@ -905,16 +927,22 @@ class ElasticsearchPlugin(application: Application) extends Plugin {
       val key = entry._1
       val curropr = entry._2
       val value = entry._3
-      for (operator <- mustOperators) {
-        if (curropr == operator) {
-          // Only add a MUST object if we have terms to populate it; empty objects break Elasticsearch
-          if (mustOperators.contains(operator) && !populatedMust) {
-            builder.startArray("must")
-            populatedMust = true
-          }
+      if (mustOperators.contains(curropr)) {
+        // Only add a MUST object if we have terms to populate it; empty objects break Elasticsearch
+        if (!populatedMust) {
+          builder.startArray("must")
+          populatedMust = true
+        }
+        builder = parseMustOperators(builder, key, value, curropr)
+      }
 
-          builder = parseMustOperators(builder, key, value, operator)
+      // For != operators, include an EXISTS query to avoid returning all documents without that field
+      if (mustNotOperators.contains(curropr)) {
+        if (!populatedMust) {
+          builder.startArray("must")
+          populatedMust = true
         }
+        builder.startObject().startObject("exists").field("field", key).endObject().endObject()
       }
     })
 
@@ -954,16 +982,13 @@ class ElasticsearchPlugin(application: Application) extends Plugin {
       val key = entry._1
       val curropr = entry._2
       val value = entry._3
-      for (operator <- mustNotOperators) {
-        if (curropr == operator) {
-          // Only add a MUST object if we have terms to populate it; empty objects break Elasticsearch
-          if (mustNotOperators.contains(operator) && !populatedMustNot) {
-            builder.startArray("must_not")
-            populatedMustNot = true
-          }
-
-          builder = parseMustNotOperators(builder, key, value, operator)
+      if (mustNotOperators.contains(curropr)) {
+        // Only add a MUST object if we have terms to populate it; empty objects break Elasticsearch
+        if (!populatedMustNot) {
+          builder.startArray("must_not")
+          populatedMustNot = true
         }
+        builder = parseMustNotOperators(builder, key, value, curropr)
       }
     })
     if (populatedMustNot) builder.endArray()

app/views/metadatald/search.scala.html

@@ -205,8 +205,10 @@ <h1>Search Metadata within Space: "@space.name"</h1>
                         "<option value='=='>equals</option>" +
                         "<option value='!='>does not equal</option>" +
                         "<option value=':'>contains</option>" +
-                        "<option value='>'>greater than</option>" +
-                        "<option value='<'>less than</option>" +
+                        //"<option value='>'>greater than</option>" +
+                        //"<option value='<'>less than</option>" +
+                        //"<option value='>='>greater or equal to</option>" +
+                        //"<option value='<='>less or equal to</option>" +
                     "</select></div>" +
                     <!-- VALUE FIELD -->
                     "<div class='form-group col-lg-4 col-md-4'>" +