Browse Source

Geo: Allow to parse lat/lon as strings and coerce them

In order to be more failsafe parsing GeoPoints can support
lat/lon as strings and coerce them. Added support and test for this.
Alexander Reelsen 11 years ago
parent
commit
8b8cd26a59

+ 16 - 8
src/main/java/org/elasticsearch/common/geo/GeoUtils.java

@@ -343,16 +343,24 @@ public class GeoUtils {
                 if(parser.currentToken() == Token.FIELD_NAME) {
                     String field = parser.text();
                     if(LATITUDE.equals(field)) {
-                        if(parser.nextToken() == Token.VALUE_NUMBER) {
-                            lat = parser.doubleValue();
-                        } else {
-                            throw new ElasticsearchParseException("latitude must be a number");
+                        parser.nextToken();
+                        switch (parser.currentToken()) {
+                            case VALUE_NUMBER:
+                            case VALUE_STRING:
+                                lat = parser.doubleValue(true);
+                                break;
+                            default:
+                                throw new ElasticsearchParseException("latitude must be a number");
                         }
                     } else if (LONGITUDE.equals(field)) {
-                        if(parser.nextToken() == Token.VALUE_NUMBER) {
-                            lon = parser.doubleValue();
-                        } else {
-                            throw new ElasticsearchParseException("latitude must be a number");
+                        parser.nextToken();
+                        switch (parser.currentToken()) {
+                            case VALUE_NUMBER:
+                            case VALUE_STRING:
+                                lon = parser.doubleValue(true);
+                                break;
+                            default:
+                                throw new ElasticsearchParseException("longitude must be a number");
                         }
                     } else if (GEOHASH.equals(field)) {
                         if(parser.nextToken() == Token.VALUE_STRING) {

+ 16 - 8
src/main/java/org/elasticsearch/search/suggest/context/GeolocationContextMapping.java

@@ -325,20 +325,28 @@ public class GeolocationContextMapping extends ContextMapping {
                 final String fieldName = parser.text();
                 if("lat".equals(fieldName)) {
                     if(point == null) {
-                        if (parser.nextToken() == Token.VALUE_NUMBER) {
-                            lat = parser.doubleValue();
-                        } else {
-                            throw new ElasticsearchParseException("latitude must be a number");
+                        parser.nextToken();
+                        switch (parser.currentToken()) {
+                            case VALUE_NUMBER:
+                            case VALUE_STRING:
+                                lat = parser.doubleValue(true);
+                                break;
+                            default:
+                                throw new ElasticsearchParseException("latitude must be a number");
                         }
                     } else {
                         throw new ElasticsearchParseException("only lat/lon or [" + FIELD_VALUE + "] is allowed");
                     }
                 } else if ("lon".equals(fieldName)) {
                     if(point == null) {
-                        if(parser.nextToken() == Token.VALUE_NUMBER) {
-                            lon = parser.doubleValue();
-                        } else {
-                            throw new ElasticsearchParseException("longitude must be a number");
+                        parser.nextToken();
+                        switch (parser.currentToken()) {
+                            case VALUE_NUMBER:
+                            case VALUE_STRING:
+                                lon = parser.doubleValue(true);
+                                break;
+                            default:
+                                throw new ElasticsearchParseException("longitude must be a number");
                         }
                     } else {
                         throw new ElasticsearchParseException("only lat/lon or [" + FIELD_VALUE + "] is allowed");

+ 15 - 1
src/test/java/org/elasticsearch/index/search/geo/GeoUtilsTests.java

@@ -26,10 +26,13 @@ import org.apache.lucene.spatial.prefix.tree.GeohashPrefixTree;
 import org.apache.lucene.spatial.prefix.tree.QuadPrefixTree;
 import org.elasticsearch.common.geo.GeoPoint;
 import org.elasticsearch.common.geo.GeoUtils;
+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 static org.hamcrest.MatcherAssert.assertThat;
+import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
 import static org.hamcrest.Matchers.*;
 
 /**
@@ -251,6 +254,17 @@ public class GeoUtilsTests extends ElasticsearchTestCase {
         }
     }
 
+    @Test
+    public void testTryParsingLatLonFromString() throws Exception {
+        XContentBuilder builder = jsonBuilder().startObject().field("lat", "52").field("lon", "4").endObject();
+        XContentParser parser = XContentHelper.createParser(builder.bytes());
+        parser.nextToken();
+        GeoPoint geoPoint = GeoUtils.parseGeoPoint(parser);
+        assertThat(geoPoint.lat(), is(52.0));
+        assertThat(geoPoint.lon(), is(4.0));
+    }
+
+
     private static void assertNormalizedPoint(GeoPoint input, GeoPoint expected) {
         GeoUtils.normalizePoint(input);
         assertThat(input, equalTo(expected));

+ 45 - 0
src/test/java/org/elasticsearch/search/suggest/context/GeoLocationContextMappingTest.java

@@ -0,0 +1,45 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+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 static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
+
+/**
+ *
+ */
+public class GeoLocationContextMappingTest extends ElasticsearchTestCase {
+
+    @Test
+    public void testThatParsingGeoPointsWorksWithCoercion() throws Exception {
+        XContentBuilder builder = jsonBuilder().startObject().field("lat", "52").field("lon", "4").endObject();
+        XContentParser parser = XContentHelper.createParser(builder.bytes());
+        parser.nextToken();
+
+        GeolocationContextMapping mapping = GeolocationContextMapping.load("foo", Maps.newHashMap());
+        mapping.parseQuery("foo", parser);
+    }
+
+}