Skip to content

Commit

Permalink
Support GeoJSON for geo_point (#85120)
Browse files Browse the repository at this point in the history 
Support GeoJSON for points when the mapper specifies geo_point. Before this, only geo_shape supported GeoJSON even for point data.

This PR also adds tests for previously existing, but not tested features like adding an array of points where each point in the array was a different format. This was working, and is now tested, and also includes the new GeoJSON format.
  • Loading branch information
@craigtaverner
craigtaverner committed Mar 23, 2022
1 parent 938f74b commit 042b964
Show file tree
Hide file tree
Showing 7 changed files with 376 additions and 90 deletions.
5 changes: 5 additions & 0 deletions docs/changelog/85120.yaml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 85120
summary: Support GeoJSON for `geo_point`
area: Geo
type: enhancement
issues: []
142 changes: 96 additions & 46 deletions server/src/main/java/org/elasticsearch/common/geo/GeoUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@
import org.elasticsearch.xcontent.support.MapXContentParser;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Locale;

public class GeoUtils {

Expand All @@ -42,6 +44,8 @@ public class GeoUtils {
public static final String LATITUDE = "lat";
public static final String LONGITUDE = "lon";
public static final String GEOHASH = "geohash";
public static final String COORDINATES = "coordinates";
public static final String TYPE = "type";

/** Earth ellipsoid major axis defined by WGS 84 in meters */
public static final double EARTH_SEMI_MAJOR_AXIS = 6378137.0; // meters (WGS 84)
Expand Down Expand Up @@ -437,75 +441,81 @@ public static GeoPoint parseGeoPoint(XContentParser parser, GeoPoint point, fina
double lat = Double.NaN;
double lon = Double.NaN;
String geohash = null;
NumberFormatException numberFormatException = null;
String geojsonType = null;
ArrayList<Double> coordinates = null;

if (parser.currentToken() == Token.START_OBJECT) {
try (XContentSubParser subParser = new XContentSubParser(parser)) {
while (subParser.nextToken() != Token.END_OBJECT) {
if (subParser.currentToken() == Token.FIELD_NAME) {
String field = subParser.currentName();
subParser.nextToken();
if (LATITUDE.equals(field)) {
subParser.nextToken();
switch (subParser.currentToken()) {
case VALUE_NUMBER:
case VALUE_STRING:
try {
lat = subParser.doubleValue(true);
} catch (NumberFormatException e) {
numberFormatException = e;
}
break;
default:
throw new ElasticsearchParseException("latitude must be a number");
}
lat = parseValidDouble(subParser, "latitude");
} else if (LONGITUDE.equals(field)) {
subParser.nextToken();
switch (subParser.currentToken()) {
case VALUE_NUMBER:
case VALUE_STRING:
try {
lon = subParser.doubleValue(true);
} catch (NumberFormatException e) {
numberFormatException = e;
}
break;
default:
throw new ElasticsearchParseException("longitude must be a number");
}
lon = parseValidDouble(subParser, "longitude");
} else if (GEOHASH.equals(field)) {
if (subParser.nextToken() == Token.VALUE_STRING) {
if (subParser.currentToken() == Token.VALUE_STRING) {
geohash = subParser.text();
} else {
throw new ElasticsearchParseException("geohash must be a string");
}
} else if (COORDINATES.equals(field)) {
if (subParser.currentToken() == Token.START_ARRAY) {
coordinates = new ArrayList<>();
while (subParser.nextToken() != Token.END_ARRAY) {
coordinates.add(parseValidDouble(subParser, field));
}
} else {
throw new ElasticsearchParseException("GeoJSON 'coordinates' must be an array");
}
} else if (TYPE.equals(field)) {
if (subParser.currentToken() == Token.VALUE_STRING) {
geojsonType = subParser.text();
} else {
throw new ElasticsearchParseException("GeoJSON 'type' must be a string");
}
} else {
throw new ElasticsearchParseException("field must be either [{}], [{}] or [{}]", LATITUDE, LONGITUDE, GEOHASH);
throw new ElasticsearchParseException(
"field must be either [{}], [{}], [{}], [{}] or [{}]",
LATITUDE,
LONGITUDE,
GEOHASH,
COORDINATES,
TYPE
);
}
} else {
throw new ElasticsearchParseException("token [{}] not allowed", subParser.currentToken());
}
}
}
assertOnlyOneFormat(
geohash != null,
Double.isNaN(lat) == false,
Double.isNaN(lon) == false,
coordinates != null,
geojsonType != null
);
if (geohash != null) {
if (Double.isNaN(lat) == false || Double.isNaN(lon) == false) {
throw new ElasticsearchParseException("field must be either lat/lon or geohash");
} else {
return point.parseGeoHash(geohash, effectivePoint);
return point.parseGeoHash(geohash, effectivePoint);
}
if (coordinates != null) {
if (geojsonType == null || geojsonType.toLowerCase(Locale.ROOT).equals("point") == false) {
throw new ElasticsearchParseException("GeoJSON 'type' for geo_point can only be 'Point'");
}
} else if (numberFormatException != null) {
throw new ElasticsearchParseException(
"[{}] and [{}] must be valid double values",
numberFormatException,
LATITUDE,
LONGITUDE
);
} else if (Double.isNaN(lat)) {
throw new ElasticsearchParseException("field [{}] missing", LATITUDE);
} else if (Double.isNaN(lon)) {
throw new ElasticsearchParseException("field [{}] missing", LONGITUDE);
} else {
return point.reset(lat, lon);
if (coordinates.size() < 2) {
throw new ElasticsearchParseException("GeoJSON 'coordinates' must contain at least two values");
}
if (coordinates.size() == 3) {
GeoPoint.assertZValue(ignoreZValue, coordinates.get(2));
}
if (coordinates.size() > 3) {
throw new ElasticsearchParseException("[geo_point] field type does not accept > 3 dimensions");
}
return point.reset(coordinates.get(1), coordinates.get(0));
}
return point.reset(lat, lon);

} else if (parser.currentToken() == Token.START_ARRAY) {
try (XContentSubParser subParser = new XContentSubParser(parser)) {
Expand Down Expand Up @@ -536,6 +546,46 @@ public static GeoPoint parseGeoPoint(XContentParser parser, GeoPoint point, fina
}
}

private static double parseValidDouble(XContentSubParser subParser, String field) throws IOException {
try {
return switch (subParser.currentToken()) {
case VALUE_NUMBER, VALUE_STRING -> subParser.doubleValue(true);
default -> throw new ElasticsearchParseException("{} must be a number", field);
};
} catch (NumberFormatException e) {
throw new ElasticsearchParseException("[{}] must be a valid double value", e, field);
}
}

private static void assertOnlyOneFormat(boolean geohash, boolean lat, boolean lon, boolean coordinates, boolean type) {
String invalidFieldsMessage = "field must be either lat/lon, geohash string or type/coordinates";
boolean latlon = lat && lon;
boolean geojson = coordinates && type;
var found = new ArrayList<String>();
if (geohash) found.add("geohash");
if (latlon) found.add("lat/lon");
if (geojson) found.add("GeoJSON");
if (found.size() > 1) {
throw new ElasticsearchParseException("fields matching more than one point format found: {}", found);
} else if (geohash) {
if (lat || lon || type || coordinates) {
throw new ElasticsearchParseException(invalidFieldsMessage);
}
} else if (found.size() == 0) {
if (lat) {
throw new ElasticsearchParseException("field [{}] missing", LONGITUDE);
} else if (lon) {
throw new ElasticsearchParseException("field [{}] missing", LATITUDE);
} else if (coordinates) {
throw new ElasticsearchParseException("field [{}] missing", TYPE);
} else if (type) {
throw new ElasticsearchParseException("field [{}] missing", COORDINATES);
} else {
throw new ElasticsearchParseException(invalidFieldsMessage);
}
}
}

/**
* Parse a {@link GeoPoint} from a string. The string must have one of the following forms:
*
Expand Down
10 changes: 10 additions & 0 deletions server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldTypeTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,16 @@ public void testFetchSourceValue() throws IOException {
assertEquals(List.of(), fetchSourceValue(mapper, sourceValue, null));
assertEquals(List.of(), fetchSourceValue(mapper, sourceValue, "wkt"));
}

// test single point in GeoJSON format
sourceValue = jsonPoint;
assertEquals(List.of(jsonPoint), fetchSourceValue(mapper, sourceValue, null));
assertEquals(List.of(wktPoint), fetchSourceValue(mapper, sourceValue, "wkt"));

// Test a list of points in GeoJSON format
sourceValue = List.of(jsonPoint, otherJsonPoint);
assertEquals(List.of(jsonPoint, otherJsonPoint), fetchSourceValue(mapper, sourceValue, null));
assertEquals(List.of(wktPoint, otherWktPoint), fetchSourceValue(mapper, sourceValue, "wkt"));
}

public void testFetchVectorTile() throws IOException {
Expand Down
81 changes: 39 additions & 42 deletions server/src/test/java/org/elasticsearch/index/search/geo/GeoPointParsingTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,18 @@

import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.geo.GeoJson;
import org.elasticsearch.common.geo.GeoPoint;
import org.elasticsearch.common.geo.GeoUtils;
import org.elasticsearch.geometry.Point;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.geo.RandomGeoGenerator;
import org.elasticsearch.xcontent.XContentBuilder;
import org.elasticsearch.xcontent.XContentParser;
import org.elasticsearch.xcontent.json.JsonXContent;

import java.io.IOException;
import java.util.HashMap;
import java.util.function.DoubleSupplier;

import static org.elasticsearch.geometry.utils.Geohash.stringEncode;
Expand Down Expand Up @@ -84,25 +87,28 @@ public void testGeoPointParsing() throws IOException {
GeoPoint point = GeoUtils.parseGeoPoint(objectLatLon(randomPt.lat(), randomPt.lon()));
assertPointsEqual(point, randomPt);

GeoUtils.parseGeoPoint(toObject(objectLatLon(randomPt.lat(), randomPt.lon())), randomBoolean());
point = GeoUtils.parseGeoPoint(toObject(objectLatLon(randomPt.lat(), randomPt.lon())), randomBoolean());
assertPointsEqual(point, randomPt);

GeoUtils.parseGeoPoint(arrayLatLon(randomPt.lat(), randomPt.lon()), point);
assertPointsEqual(point, randomPt);

GeoUtils.parseGeoPoint(toObject(arrayLatLon(randomPt.lat(), randomPt.lon())), randomBoolean());
point = GeoUtils.parseGeoPoint(toObject(arrayLatLon(randomPt.lat(), randomPt.lon())), randomBoolean());
assertPointsEqual(point, randomPt);

GeoUtils.parseGeoPoint(geohash(randomPt.lat(), randomPt.lon()), point);
assertCloseTo(point, randomPt.lat(), randomPt.lon());

GeoUtils.parseGeoPoint(toObject(geohash(randomPt.lat(), randomPt.lon())), randomBoolean());
point = GeoUtils.parseGeoPoint(toObject(geohash(randomPt.lat(), randomPt.lon())), randomBoolean());
assertCloseTo(point, randomPt.lat(), randomPt.lon());

GeoUtils.parseGeoPoint(stringLatLon(randomPt.lat(), randomPt.lon()), point);
assertCloseTo(point, randomPt.lat(), randomPt.lon());

GeoUtils.parseGeoPoint(toObject(stringLatLon(randomPt.lat(), randomPt.lon())), randomBoolean());
point = GeoUtils.parseGeoPoint(toObject(stringLatLon(randomPt.lat(), randomPt.lon())), randomBoolean());
assertCloseTo(point, randomPt.lat(), randomPt.lon());

point = GeoUtils.parseGeoPoint(GeoJson.toMap(new Point(randomPt.lon(), randomPt.lat())), randomBoolean());
assertCloseTo(point, randomPt.lat(), randomPt.lon());
}

Expand All @@ -118,49 +124,40 @@ public void testInvalidPointEmbeddedObject() throws IOException {
try (XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(content))) {
parser.nextToken();
Exception e = expectThrows(ElasticsearchParseException.class, () -> GeoUtils.parseGeoPoint(parser));
assertThat(e.getMessage(), is("field must be either [lat], [lon] or [geohash]"));
}
try (XContentParser parser2 = createParser(JsonXContent.jsonXContent, BytesReference.bytes(content))) {
parser2.nextToken();
Exception e = expectThrows(ElasticsearchParseException.class, () -> GeoUtils.parseGeoPoint(toObject(parser2), randomBoolean()));
assertThat(e.getMessage(), is("field must be either [lat], [lon] or [geohash]"));
}
}

public void testInvalidPointLatHashMix() throws IOException {
XContentBuilder content = JsonXContent.contentBuilder();
content.startObject();
content.field("lat", 0).field("geohash", stringEncode(0d, 0d));
content.endObject();

try (XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(content))) {
parser.nextToken();
Exception e = expectThrows(ElasticsearchParseException.class, () -> GeoUtils.parseGeoPoint(parser));
assertThat(e.getMessage(), is("field must be either lat/lon or geohash"));
assertThat(e.getMessage(), is("field must be either [lat], [lon], [geohash], [coordinates] or [type]"));
}
try (XContentParser parser2 = createParser(JsonXContent.jsonXContent, BytesReference.bytes(content))) {
parser2.nextToken();
Exception e = expectThrows(ElasticsearchParseException.class, () -> GeoUtils.parseGeoPoint(toObject(parser2), randomBoolean()));
assertThat(e.getMessage(), is("field must be either lat/lon or geohash"));
assertThat(e.getMessage(), is("field must be either [lat], [lon], [geohash], [coordinates] or [type]"));
}
}

public void testInvalidPointLonHashMix() throws IOException {
XContentBuilder content = JsonXContent.contentBuilder();
content.startObject();
content.field("lon", 0).field("geohash", stringEncode(0d, 0d));
content.endObject();

try (XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(content))) {
parser.nextToken();

Exception e = expectThrows(ElasticsearchParseException.class, () -> GeoUtils.parseGeoPoint(parser));
assertThat(e.getMessage(), is("field must be either lat/lon or geohash"));
}
try (XContentParser parser2 = createParser(JsonXContent.jsonXContent, BytesReference.bytes(content))) {
parser2.nextToken();
Exception e = expectThrows(ElasticsearchParseException.class, () -> GeoUtils.parseGeoPoint(toObject(parser2), randomBoolean()));
assertThat(e.getMessage(), is("field must be either lat/lon or geohash"));
public void testInvalidPointHashMix() throws IOException {
HashMap<String, Object> otherFields = new HashMap<>();
otherFields.put("lat", 0);
otherFields.put("lon", 0);
otherFields.put("type", "Point");
otherFields.put("coordinates", new double[] { 0.0, 0.0 });
for (String other : otherFields.keySet()) {
XContentBuilder content = JsonXContent.contentBuilder();
content.startObject();
content.field(other, otherFields.get(other)).field("geohash", stringEncode(0d, 0d));
content.endObject();

try (XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(content))) {
parser.nextToken();
Exception e = expectThrows(ElasticsearchParseException.class, () -> GeoUtils.parseGeoPoint(parser));
assertThat(e.getMessage(), is("field must be either lat/lon, geohash string or type/coordinates"));
}
try (XContentParser parser2 = createParser(JsonXContent.jsonXContent, BytesReference.bytes(content))) {
parser2.nextToken();
Exception e = expectThrows(
ElasticsearchParseException.class,
() -> GeoUtils.parseGeoPoint(toObject(parser2), randomBoolean())
);
assertThat(e.getMessage(), is("field must be either lat/lon, geohash string or type/coordinates"));
}
}
}

Expand All @@ -173,13 +170,13 @@ public void testInvalidField() throws IOException {
try (XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(content))) {
parser.nextToken();
Exception e = expectThrows(ElasticsearchParseException.class, () -> GeoUtils.parseGeoPoint(parser));
assertThat(e.getMessage(), is("field must be either [lat], [lon] or [geohash]"));
assertThat(e.getMessage(), is("field must be either [lat], [lon], [geohash], [coordinates] or [type]"));
}

try (XContentParser parser2 = createParser(JsonXContent.jsonXContent, BytesReference.bytes(content))) {
parser2.nextToken();
Exception e = expectThrows(ElasticsearchParseException.class, () -> GeoUtils.parseGeoPoint(toObject(parser2), randomBoolean()));
assertThat(e.getMessage(), is("field must be either [lat], [lon] or [geohash]"));
assertThat(e.getMessage(), is("field must be either [lat], [lon], [geohash], [coordinates] or [type]"));
}
}

Expand Down

0 comments on commit 042b964

Please sign in to comment.