ContextSuggester: Adding couple of tests to catch more bugs

A bunch of minor fixes have been included here, especially due
to wrongly parsed mappings. Also using assertions resulted in an
NPE because they were disabled in the distribution.

Closes #5525

rest-api-spec/test/suggest/20_context.yaml

@@ -0,0 +1,219 @@
+# This test creates one huge mapping in the setup
+# Every test should use its own field to make sure it works
+
+setup:
+
+  - do:
+      indices.create:
+          index: test
+          body:
+            mappings:
+              test:
+                "properties":
+                  "suggest_context":
+                     "type" : "completion"
+                     "context":
+                        "color":
+                            "type" : "category"
+                  "suggest_context_default_hardcoded":
+                     "type" : "completion"
+                     "context":
+                        "color":
+                            "type" : "category"
+                            "default" : "red"
+                  "suggest_context_default_path":
+                     "type" : "completion"
+                     "context":
+                        "color":
+                            "type" : "category"
+                            "default" : "red"
+                            "path" : "color"
+                  "suggest_geo":
+                     "type" : "completion"
+                     "context":
+                        "location":
+                            "type" : "geo"
+
+---
+"Simple context suggestion should work":
+
+  - do:
+      index:
+        index: test
+        type:  test
+        id:    1
+        body:
+          suggest_context:
+            input: "Hoodie red"
+            context:
+              color: "red"
+
+  - do:
+      index:
+        index: test
+        type:  test
+        id:    2
+        body:
+          suggest_context:
+            input: "Hoodie blue"
+            context:
+              color: "blue"
+
+  - do:
+      indices.refresh: {}
+
+  - do:
+      suggest:
+        body:
+          result:
+            text: "hoo"
+            completion:
+              field: suggest_context
+              context:
+                color: "red"
+
+  - match: {result.0.options.0.text: "Hoodie red" }
+
+---
+"Hardcoded category value should work":
+
+  - do:
+      index:
+        index: test
+        type:  test
+        id:    1
+        body:
+          suggest_context_default_hardcoded:
+            input: "Hoodie red"
+
+  - do:
+      index:
+        index: test
+        type:  test
+        id:    2
+        body:
+          suggest_context_default_hardcoded:
+            input: "Hoodie blue"
+            context:
+              color: "blue"
+
+  - do:
+      indices.refresh: {}
+
+  - do:
+      suggest:
+        body:
+          result:
+            text: "hoo"
+            completion:
+              field: suggest_context_default_hardcoded
+              context:
+                color: "red"
+
+  - length: { result: 1  }
+  - length: { result.0.options: 1  }
+  - match:  { result.0.options.0.text: "Hoodie red" }
+
+
+---
+"Category suggest context default path should work":
+
+  - do:
+      index:
+        index: test
+        type:  test
+        id:    1
+        body:
+          suggest_context_default_path:
+            input: "Hoodie red"
+
+  - do:
+      index:
+        index: test
+        type:  test
+        id:    2
+        body:
+          suggest_context_default_path:
+            input: "Hoodie blue"
+          color: "blue"
+
+  - do:
+      indices.refresh: {}
+
+  - do:
+      suggest:
+        body:
+          result:
+            text: "hoo"
+            completion:
+              field: suggest_context_default_path
+              context:
+                color: "red"
+
+  - length: { result: 1  }
+  - length: { result.0.options: 1  }
+  - match:  { result.0.options.0.text: "Hoodie red" }
+
+  - do:
+      suggest:
+        body:
+          result:
+            text: "hoo"
+            completion:
+              field: suggest_context_default_path
+              context:
+                color: "blue"
+
+  - length: { result: 1  }
+  - length: { result.0.options: 1  }
+  - match:  { result.0.options.0.text: "Hoodie blue" }
+
+
+---
+"Geo suggest should work":
+
+  - do:
+      index:
+        index: test
+        type:  test
+        id:    1
+        body:
+          suggest_geo:
+            input: "Hotel Marriot in Amsterdam"
+            context:
+              location:
+                lat : 52.22
+                lon : 4.53
+
+  - do:
+      index:
+        index: test
+        type:  test
+        id:    2
+        body:
+          suggest_geo:
+            input: "Hotel Marriot in Berlin"
+            context:
+              location:
+                lat : 53.31
+                lon : 13.24
+
+  - do:
+      indices.refresh: {}
+
+  - do:
+      suggest:
+        body:
+          result:
+            text: "hote"
+            completion:
+              field: suggest_geo
+              context:
+                location:
+                  lat : 52.22
+                  lon : 4.53
+
+  - length: { result: 1  }
+  - length: { result.0.options: 1  }
+  - match:  { result.0.options.0.text: "Hotel Marriot in Amsterdam" }
+

src/main/java/org/apache/lucene/analysis/PrefixAnalyzer.java

@@ -21,6 +21,7 @@
 
 import org.apache.lucene.analysis.tokenattributes.CharTermAttribute;
 import org.apache.lucene.analysis.tokenattributes.PositionIncrementAttribute;
+import org.elasticsearch.ElasticsearchIllegalArgumentException;
 
 import java.io.IOException;
 import java.io.Reader;
@@ -96,7 +97,9 @@ public PrefixTokenFilter(TokenStream input, char separator, Iterable<? extends C
             this.prefixes = prefixes;
             this.currentPrefix = null;
             this.separator = separator;
-            assert (prefixes != null && prefixes.iterator().hasNext()) : "one or more prefix needed";
+            if (prefixes == null || !prefixes.iterator().hasNext()) {
+                throw new ElasticsearchIllegalArgumentException("one or more prefixes needed");
+            }
         }
 
         @Override

src/main/java/org/elasticsearch/search/suggest/SuggestBuilder.java

@@ -33,7 +33,7 @@
 
 /**
  * Defines how to perform suggesting. This builders allows a number of global options to be specified and
- * an arbitrary number of {@link org.elasticsearch.search.suggest.SuggestBuilder.TermSuggestionBuilder} instances.
+ * an arbitrary number of {@link org.elasticsearch.search.suggest.term.TermSuggestionBuilder} instances.
  * <p/>
  * Suggesting works by suggesting terms that appear in the suggest text that are similar compared to the terms in
  * provided text. These spelling suggestions are based on several options described in this class.
@@ -66,7 +66,7 @@ public SuggestBuilder setText(String globalText) {
     }
 
     /**
-     * Adds an {@link org.elasticsearch.search.suggest.SuggestBuilder.TermSuggestionBuilder} instance under a user defined name.
+     * Adds an {@link org.elasticsearch.search.suggest.term.TermSuggestionBuilder} instance under a user defined name.
      * The order in which the <code>Suggestions</code> are added, is the same as in the response.
      */
     public SuggestBuilder addSuggestion(SuggestionBuilder<?> suggestion) {
@@ -141,17 +141,28 @@ private T addContextQuery(ContextQuery ctx) {
         }
 
         /**
-         * Setup a Geolocation for suggestions. See {@link GeoContextMapping}.
+         * Setup a Geolocation for suggestions. See {@link GeolocationContextMapping}.
          * @param lat Latitude of the location
          * @param lon Longitude of the Location
          * @return this
          */
-        public T addGeoLocation(String name, double lat, double lon) {
-            return addContextQuery(GeolocationContextMapping.query(name, lat, lon));
+        public T addGeoLocation(String name, double lat, double lon, int ... precisions) {
+            return addContextQuery(GeolocationContextMapping.query(name, lat, lon, precisions));
         }
 
         /**
-         * Setup a Geolocation for suggestions. See {@link GeoContextMapping}.
+         * Setup a Geolocation for suggestions. See {@link GeolocationContextMapping}.
+         * @param lat Latitude of the location
+         * @param lon Longitude of the Location
+         * @param precisions precisions as string var-args
+         * @return this
+         */
+        public T addGeoLocationWithPrecision(String name, double lat, double lon, String ... precisions) {
+            return addContextQuery(GeolocationContextMapping.query(name, lat, lon, precisions));
+        }
+
+        /**
+         * Setup a Geolocation for suggestions. See {@link GeolocationContextMapping}.
          * @param geohash Geohash of the location
          * @return this
          */
@@ -160,17 +171,17 @@ public T addGeoLocation(String name, String geohash) {
         }
 
         /**
-         * Setup a Category for suggestions. See {@link CategoryMapping}.
-         * @param category name of the category
+         * Setup a Category for suggestions. See {@link CategoryContextMapping}.
+         * @param categories name of the category
          * @return this
          */
         public T addCategory(String name, CharSequence...categories) {
             return addContextQuery(CategoryContextMapping.query(name, categories));
         }
 
         /**
-         * Setup a Category for suggestions. See {@link CategoryMapping}.
-         * @param category name of the category
+         * Setup a Category for suggestions. See {@link CategoryContextMapping}.
+         * @param categories name of the category
          * @return this
          */
         public T addCategory(String name, Iterable<? extends CharSequence> categories) {
@@ -179,7 +190,7 @@ public T addCategory(String name, Iterable<? extends CharSequence> categories) {
 
         /**
          * Setup a Context Field for suggestions. See {@link CategoryContextMapping}.
-         * @param category name of the category
+         * @param fieldvalues name of the category
          * @return this
          */
         public T addContextField(String name, CharSequence...fieldvalues) {
@@ -188,7 +199,7 @@ public T addContextField(String name, CharSequence...fieldvalues) {
 
         /**
          * Setup a Context Field for suggestions. See {@link CategoryContextMapping}.
-         * @param category name of the category
+         * @param fieldvalues name of the category
          * @return this
          */
         public T addContextField(String name, Iterable<? extends CharSequence> fieldvalues) {
@@ -242,7 +253,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
         /**
          * Sets from what field to fetch the candidate suggestions from. This is an
          * required option and needs to be set via this setter or
-         * {@link org.elasticsearch.search.suggest.SuggestBuilder.TermSuggestionBuilder#setField(String)}
+         * {@link org.elasticsearch.search.suggest.term.TermSuggestionBuilder#field(String)}
          * method
          */
         @SuppressWarnings("unchecked")

src/main/java/org/elasticsearch/search/suggest/context/CategoryContextMapping.java

@@ -19,6 +19,7 @@
 
 package org.elasticsearch.search.suggest.context;
 
+import com.google.common.base.Joiner;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import org.apache.lucene.analysis.PrefixAnalyzer;
@@ -235,29 +236,30 @@ public FieldConfig(String fieldname, Iterable<? extends CharSequence> defaultVal
 
         @Override
         protected TokenStream wrapTokenStream(Document doc, TokenStream stream) {
-            if(values != null) {
+            if (values != null) {
                 return new PrefixAnalyzer.PrefixTokenFilter(stream, ContextMapping.SEPARATOR, values);
+            // if fieldname is default, BUT our default values are set, we take that one
+            } else if ((doc.getFields(fieldname).length == 0 || fieldname.equals(DEFAULT_FIELDNAME)) && defaultValues.iterator().hasNext()) {
+                return new PrefixAnalyzer.PrefixTokenFilter(stream, ContextMapping.SEPARATOR, defaultValues);
             } else {
                 IndexableField[] fields = doc.getFields(fieldname);
                 ArrayList<CharSequence> values = new ArrayList<>(fields.length);
-
                 for (int i = 0; i < fields.length; i++) {
                     values.add(fields[i].stringValue());
                 }
-    
+
                 return new PrefixAnalyzer.PrefixTokenFilter(stream, ContextMapping.SEPARATOR, values);
             }
         }
 
         @Override
         public String toString() {
             StringBuilder sb = new StringBuilder("FieldConfig(" + fieldname + " = [");
-            Iterator<? extends CharSequence> value = this.defaultValues.iterator();
-            if (value.hasNext()) {
-                sb.append(value.next());
-                while (value.hasNext()) {
-                    sb.append(", ").append(value.next());
-                }
+            if (this.values != null && this.values.iterator().hasNext()) {
+                sb.append("(").append(Joiner.on(", ").join(this.values.iterator())).append(")");
+            }
+            if (this.defaultValues != null && this.defaultValues.iterator().hasNext()) {
+                sb.append(" default(").append(Joiner.on(", ").join(this.defaultValues.iterator())).append(")");
             }
             return sb.append("])").toString();
         }