Geo context suggester: Require precision in mapping

The default precision was way too exact and could lead people to
think that geo context suggestions are not working. This patch now
requires you to set the precision in the mapping, as elasticsearch itself
can never tell exactly, what the required precision for the users
suggestions are.

Closes #5621

docs/reference/search/suggesters/context-suggest.asciidoc

@@ -174,13 +174,12 @@ geohash of a certain precision, which provides the context.
 
 [float]
 ==== Geo location Mapping
-The mapping for a geo context accepts four settings:
+The mapping for a geo context accepts four settings, only of which `precision` is required:
 
 [horizontal]
 `precision`::  This defines the precision of the geohash and can be specified as `5m`, `10km`,
                or as a raw geohash precision: `1`..`12`. It's also possible to setup multiple
                precisions by defining a list of precisions: `["5m", "10km"]`
-               (default is a geohash level of 12)
 `neighbors`::  Geohashes are rectangles, so a geolocation, which in reality is only 1 metre
                away from the specified point, may fall into the neighbouring rectangle. Set
                `neighbours` to `true` to include the neighbouring geohashes in the context.

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

@@ -33,6 +33,7 @@ setup:
                      "context":
                         "location":
                             "type" : "geo"
+                            "precision" : "5km"
 
 ---
 "Simple context suggestion should work":
@@ -201,8 +202,12 @@ setup:
   - do:
       indices.refresh: {}
 
+  - do:
+      indices.get_mapping: {}
+
   - do:
       suggest:
+        index: test
         body:
           result:
             text: "hote"

src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggestionBuilder.java

@@ -18,8 +18,6 @@
  */
 package org.elasticsearch.search.suggest.completion;
 
-import org.apache.lucene.search.suggest.analyzing.XFuzzySuggester;
-import org.elasticsearch.common.xcontent.ToXContent;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.search.suggest.SuggestBuilder;
 

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

@@ -114,6 +114,10 @@ public class GeolocationContextMapping extends ContextMapping {
      *         <code>config</code>
      */
     protected static GeolocationContextMapping load(String name, Map<String, Object> config) {
+        if (!config.containsKey(FIELD_PRECISION)) {
+            throw new ElasticsearchParseException("field [precision] is missing");
+        }
+
         final GeolocationContextMapping.Builder builder = new GeolocationContextMapping.Builder(name);
 
         if (config != null) {
@@ -381,6 +385,10 @@ public class GeolocationContextMapping extends ContextMapping {
                 }
             }
 
+            if (precision == null || precision.length == 0) {
+                precision = this.precision;
+            }
+
             return new GeoQuery(name, point.geohash(), precision);
         } else {
             return new GeoQuery(name, GeoUtils.parseGeoPoint(parser).getGeohash(), precision);

src/test/java/org/elasticsearch/search/suggest/ContextSuggestSearchTests.java

@@ -27,7 +27,9 @@ import org.elasticsearch.action.suggest.SuggestResponse;
 import org.elasticsearch.common.geo.GeoHashUtils;
 import org.elasticsearch.common.geo.GeoPoint;
 import org.elasticsearch.common.unit.Fuzziness;
-import org.elasticsearch.common.xcontent.XContentBuilder;
+import org.elasticsearch.common.xcontent.*;
+import org.elasticsearch.index.mapper.MapperParsingException;
+import org.elasticsearch.search.aggregations.support.format.ValueFormatter;
 import org.elasticsearch.search.suggest.Suggest.Suggestion;
 import org.elasticsearch.search.suggest.Suggest.Suggestion.Entry;
 import org.elasticsearch.search.suggest.Suggest.Suggestion.Entry.Option;
@@ -479,6 +481,7 @@ public class ContextSuggestSearchTests extends ElasticsearchIntegrationTest {
                 .field("type", "completion")
                 .startObject("context").startObject("location")
                     .field("type", "geo")
+                    .field("precision", "500m")
                     .startObject("default").field("lat", berlinAlexanderplatz.lat()).field("lon", berlinAlexanderplatz.lon()).endObject()
                 .endObject().endObject()
                 .endObject().endObject().endObject()
@@ -529,6 +532,7 @@ public class ContextSuggestSearchTests extends ElasticsearchIntegrationTest {
                 .field("type", "completion")
                 .startObject("context").startObject("location")
                     .field("type", "geo")
+                    .field("precision", "1m")
                 .endObject().endObject()
             .endObject().endObject().endObject()
             .endObject();
@@ -670,6 +674,7 @@ public class ContextSuggestSearchTests extends ElasticsearchIntegrationTest {
                 .field("type", "completion")
                 .startObject("context").startObject("location")
                 .field("type", "geo")
+                .field("precision", "5m")
                 .field("path", "loc")
                 .endObject().endObject()
                 .endObject().endObject().endObject()
@@ -687,6 +692,57 @@ public class ContextSuggestSearchTests extends ElasticsearchIntegrationTest {
         assertSuggestion(suggestResponse.getSuggest(), 0, "suggestion", "Berlin Alexanderplatz");
     }
 
+    @Test(expected = MapperParsingException.class)
+    public void testThatPrecisionIsRequired() throws Exception {
+        XContentBuilder xContentBuilder = jsonBuilder().startObject()
+                .startObject("item").startObject("properties").startObject("suggest")
+                .field("type", "completion")
+                .startObject("context").startObject("location")
+                .field("type", "geo")
+                .field("path", "loc")
+                .endObject().endObject()
+                .endObject().endObject().endObject()
+                .endObject();
+
+        assertAcked(prepareCreate(INDEX).addMapping("item", xContentBuilder));
+    }
+
+    @Test
+    public void testThatLatLonParsingFromSourceWorks() throws Exception {
+        XContentBuilder xContentBuilder = jsonBuilder().startObject()
+                .startObject("mappings").startObject("test").startObject("properties").startObject("suggest_geo")
+                .field("type", "completion")
+                .startObject("context").startObject("location")
+                .field("type", "geo")
+                .field("precision", "1km")
+                .endObject().endObject()
+                .endObject().endObject().endObject()
+                .endObject().endObject();
+
+        assertAcked(prepareCreate("test").setSource(xContentBuilder.bytes()));
+
+        double latitude = 52.22;
+        double longitude = 4.53;
+        String geohash = GeoHashUtils.encode(latitude, longitude);
+
+        XContentBuilder doc1 = jsonBuilder().startObject().startObject("suggest_geo").field("input", "Hotel Marriot in Amsterdam").startObject("context").startObject("location").field("lat", latitude).field("lon", longitude).endObject().endObject().endObject().endObject();
+        index("test", "test", "1", doc1);
+        XContentBuilder doc2 = jsonBuilder().startObject().startObject("suggest_geo").field("input", "Hotel Marriot in Berlin").startObject("context").startObject("location").field("lat", 53.31).field("lon", 13.24).endObject().endObject().endObject().endObject();
+        index("test", "test", "2", doc2);
+        refresh();
+
+        XContentBuilder source = jsonBuilder().startObject().startObject("suggestion").field("text", "h").startObject("completion").field("field", "suggest_geo").startObject("context").field("location", geohash).endObject().endObject().endObject().endObject();
+        SuggestRequest suggestRequest = new SuggestRequest(INDEX).suggest(source.bytes());
+        SuggestResponse suggestResponse = client().suggest(suggestRequest).get();
+        assertSuggestion(suggestResponse.getSuggest(), 0, "suggestion", "Hotel Marriot in Amsterdam");
+
+        // this is exact the same request, but using lat/lon instead of geohash
+        source = jsonBuilder().startObject().startObject("suggestion").field("text", "h").startObject("completion").field("field", "suggest_geo").startObject("context").startObject("location").field("lat", latitude).field("lon", longitude).endObject().endObject().endObject().endObject().endObject();
+        suggestRequest = new SuggestRequest(INDEX).suggest(source.bytes());
+        suggestResponse = client().suggest(suggestRequest).get();
+        assertSuggestion(suggestResponse.getSuggest(), 0, "suggestion", "Hotel Marriot in Amsterdam");
+    }
+
     public void assertGeoSuggestionsInRange(String location, String suggest, double precision) throws IOException {
         String suggestionName = randomAsciiOfLength(10);
         CompletionSuggestionBuilder context = new CompletionSuggestionBuilder(suggestionName).field(FIELD).text(suggest).size(10)

src/test/java/org/elasticsearch/search/suggest/context/GeoLocationContextMappingTest.java

@@ -18,13 +18,14 @@
  */
 package org.elasticsearch.search.suggest.context;
 
-import com.carrotsearch.ant.tasks.junit4.dependencies.com.google.common.collect.Maps;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentHelper;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.test.ElasticsearchTestCase;
 import org.junit.Test;
 
+import java.util.HashMap;
+
 import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
 
 /**
@@ -38,7 +39,9 @@ public class GeoLocationContextMappingTest extends ElasticsearchTestCase {
         XContentParser parser = XContentHelper.createParser(builder.bytes());
         parser.nextToken();
 
-        GeolocationContextMapping mapping = GeolocationContextMapping.load("foo", Maps.newHashMap());
+        HashMap<String, Object> config = new HashMap<>();
+        config.put("precision", 12);
+        GeolocationContextMapping mapping = GeolocationContextMapping.load("foo", config);
         mapping.parseQuery("foo", parser);
     }