diff --git a/CHANGELOG.md b/CHANGELOG.md index b6af9b7041db3..4323ecaa64fc9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Add BWC version 2.3.1 ([#4513](https://github.com/opensearch-project/OpenSearch/pull/4513)) - [Segment Replication] Add snapshot and restore tests for segment replication feature ([#3993](https://github.com/opensearch-project/OpenSearch/pull/3993)) - Added missing javadocs for `:example-plugins` modules ([#4540](https://github.com/opensearch-project/OpenSearch/pull/4540)) +- Add support for GeoJson Point type in GeoPoint field ([#4597](https://github.com/opensearch-project/OpenSearch/pull/4597)) ### Dependencies - Bumps `log4j-core` from 2.18.0 to 2.19.0 diff --git a/server/src/main/java/org/opensearch/common/geo/GeoPoint.java b/server/src/main/java/org/opensearch/common/geo/GeoPoint.java index a2b06dccded8c..f6b049b46789d 100644 --- a/server/src/main/java/org/opensearch/common/geo/GeoPoint.java +++ b/server/src/main/java/org/opensearch/common/geo/GeoPoint.java @@ -119,7 +119,11 @@ public GeoPoint resetFromString(String value, final boolean ignoreZValue, Effect public GeoPoint resetFromCoordinates(String value, final boolean ignoreZValue) { String[] vals = value.split(","); if (vals.length > 3) { - throw new OpenSearchParseException("failed to parse [{}], expected 2 or 3 coordinates " + "but found: [{}]", vals.length); + throw new OpenSearchParseException( + "failed to parse [{}], expected 2 or 3 coordinates " + "but found: [{}]", + vals.length, + vals.length + ); } final double lat; final double lon; diff --git a/server/src/main/java/org/opensearch/common/geo/GeoUtils.java b/server/src/main/java/org/opensearch/common/geo/GeoUtils.java index 5534967d559d6..8c0cf9ca551bd 100644 --- a/server/src/main/java/org/opensearch/common/geo/GeoUtils.java +++ b/server/src/main/java/org/opensearch/common/geo/GeoUtils.java @@ -40,10 +40,9 @@ import org.opensearch.common.xcontent.LoggingDeprecationHandler; import org.opensearch.common.xcontent.NamedXContentRegistry; import org.opensearch.common.xcontent.XContentParser; -import org.opensearch.common.xcontent.XContentParser.Token; -import org.opensearch.common.xcontent.XContentSubParser; import org.opensearch.common.xcontent.support.MapXContentParser; import org.opensearch.common.xcontent.support.XContentMapValues; +import org.opensearch.geometry.Point; import org.opensearch.index.fielddata.FieldData; import org.opensearch.index.fielddata.GeoPointValues; import org.opensearch.index.fielddata.MultiGeoPointValues; @@ -95,6 +94,12 @@ public class GeoUtils { /** rounding error for quantized latitude and longitude values */ public static final double TOLERANCE = 1E-6; + public static final PointParser POINT_PARSER; + + static { + POINT_PARSER = new PointParser(LONGITUDE, LATITUDE, true); + } + /** Returns true if latitude is actually a valid latitude value.*/ public static boolean isValidLatitude(double latitude) { if (Double.isNaN(latitude) || Double.isInfinite(latitude) || latitude < GeoUtils.MIN_LAT || latitude > GeoUtils.MAX_LAT) { @@ -444,113 +449,41 @@ public static GeoPoint parseGeoPoint(XContentParser parser, GeoPoint point, fina * Parse a {@link GeoPoint} with a {@link XContentParser}. A geopoint has one of the following forms: * *
{"lat": <latitude>, "lon": <longitude>}
"<latitude>,<longitude>"
"<geohash>"
[<longitude>,<latitude>]
{@code {"lat":, "lon":
{@code ", "}
{@code ""}
{@code "POINT ()"}
{@code [, ]}
{@code {"type": "Point", "coordinates": [, ]}}
parse(XContentParser parser) throws IOException, ParseException { - - if (parser.currentToken() == XContentParser.Token.START_ARRAY) { - XContentParser.Token token = parser.nextToken(); - P point = pointSupplier.get(); + if (parser.currentToken() == XContentParser.Token.START_ARRAY && parser.nextToken() != XContentParser.Token.VALUE_NUMBER) { + // Array of points ArrayList
points = new ArrayList<>(); - if (token == XContentParser.Token.VALUE_NUMBER) { - double x = parser.doubleValue(); + while (parser.currentToken() != XContentParser.Token.END_ARRAY) { + points.add(process(objectParser.apply(parser, pointSupplier.get()))); parser.nextToken(); - double y = parser.doubleValue(); - token = parser.nextToken(); - if (token == XContentParser.Token.VALUE_NUMBER) { - GeoPoint.assertZValue(ignoreZValue, parser.doubleValue()); - } else if (token != XContentParser.Token.END_ARRAY) { - throw new OpenSearchParseException("field type does not accept > 3 dimensions"); - } - - point.resetCoords(x, y); - points.add(process(point)); - } else { - while (token != XContentParser.Token.END_ARRAY) { - points.add(process(objectParser.apply(parser, point))); - point = pointSupplier.get(); - token = parser.nextToken(); - } } return points; } else if (parser.currentToken() == XContentParser.Token.VALUE_NULL) { + // Null value if (nullValue == null) { return null; } else { return Collections.singletonList(nullValue); } } else { + // Single point return Collections.singletonList(process(objectParser.apply(parser, pointSupplier.get()))); } } diff --git a/server/src/test/java/org/opensearch/common/geo/PointParserTests.java b/server/src/test/java/org/opensearch/common/geo/PointParserTests.java new file mode 100644 index 0000000000000..dd90ed7636612 --- /dev/null +++ b/server/src/test/java/org/opensearch/common/geo/PointParserTests.java @@ -0,0 +1,205 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.common.geo; + +import org.opensearch.OpenSearchParseException; +import org.opensearch.common.xcontent.XContentBuilder; +import org.opensearch.common.xcontent.XContentFactory; +import org.opensearch.common.xcontent.XContentParser; +import org.opensearch.geometry.Point; +import org.opensearch.test.OpenSearchTestCase; + +import java.io.IOException; + +public class PointParserTests extends OpenSearchTestCase { + private static final PointParser PARSER; + private static final PointParser PARSER_WITHOUT_GEOHASH; + private static final String FIELD_NAME_X = "x"; + private static final String FIELD_NAME_Y = "y"; + private static final String FN_GEOHASH = "geohash"; + private static final String FN_GEOJSON_TYPE = "type"; + private static final String FV_GEOJSON_TYPE_POINT = "Point"; + private static final String FN_GEOJSON_COORDS = "coordinates"; + + private static final String FN_IN_ERR_MSG = "test error message"; + + static { + PARSER = new PointParser(FIELD_NAME_X, FIELD_NAME_Y, true); + PARSER_WITHOUT_GEOHASH = new PointParser(FIELD_NAME_X, FIELD_NAME_Y, false); + } + + public void testParseFromObjectOfBasicForm() throws IOException { + double x = 74.00; + double y = 40.71; + XContentBuilder pointInJson = XContentFactory.jsonBuilder().startObject().field(FIELD_NAME_X, x).field(FIELD_NAME_Y, y).endObject(); + try (XContentParser parser = createParser(pointInJson)) { + parser.nextToken(); + Point point = PARSER.parseObject(parser, true, GeoUtils.EffectivePoint.BOTTOM_LEFT); + assertEquals(x, point.getX(), 0.001); + assertEquals(y, point.getY(), 0.001); + } + } + + public void testParseFromObjectOfBasicFormMissingValue() throws IOException { + double x = 74.00; + XContentBuilder pointInJson = XContentFactory.jsonBuilder().startObject().field(FIELD_NAME_X, x).endObject(); + try (XContentParser parser = createParser(pointInJson)) { + parser.nextToken(); + Point point = PARSER.parseObject(parser, true, GeoUtils.EffectivePoint.BOTTOM_LEFT); + assertTrue("Expect an exception", false); + } catch (OpenSearchParseException e) { + // Expected exception + } + } + + public void testParseFromObjectOfBasicFormAdditionalField() throws IOException { + double x = 74.00; + double y = 40.71; + XContentBuilder pointInJson = XContentFactory.jsonBuilder() + .startObject() + .field(FIELD_NAME_X, x) + .field(FIELD_NAME_Y, y) + .field("InvalidField", 10.1) + .endObject(); + try (XContentParser parser = createParser(pointInJson)) { + parser.nextToken(); + Point point = PARSER.parseObject(parser, true, GeoUtils.EffectivePoint.BOTTOM_LEFT); + assertTrue("Expect an exception", false); + } catch (OpenSearchParseException e) { + // Expected exception + } + } + + public void testParseFromObjectOfGeoHash() throws IOException { + double longitude = 74.00; + double latitude = 40.71; + String geohash = "txhxegj0uyp3"; + XContentBuilder pointInJson = XContentFactory.jsonBuilder().startObject().field(FN_GEOHASH, geohash).endObject(); + try (XContentParser parser = createParser(pointInJson)) { + parser.nextToken(); + Point point = PARSER.parseObject(parser, true, GeoUtils.EffectivePoint.BOTTOM_LEFT); + assertEquals(longitude, point.getLon(), 0.001); + assertEquals(latitude, point.getLat(), 0.001); + } + } + + public void testParseFromObjectOfGeoHashWithNoGeoHashSupport() throws IOException { + String geohash = "txhxegj0uyp3"; + XContentBuilder pointInJson = XContentFactory.jsonBuilder().startObject().field(FN_GEOHASH, geohash).endObject(); + try (XContentParser parser = createParser(pointInJson)) { + parser.nextToken(); + PARSER_WITHOUT_GEOHASH.parseObject(parser, true, GeoUtils.EffectivePoint.BOTTOM_LEFT); + assertTrue("Expected an exception", false); + } catch (OpenSearchParseException e) { + // Expected exception + } + } + + public void testParseFromObjectOfGeoJson() throws IOException { + double x = 41.12; + double y = -71.34; + XContentBuilder pointInJson = XContentFactory.jsonBuilder() + .startObject() + .field(FN_GEOJSON_TYPE, FV_GEOJSON_TYPE_POINT) + .array(FN_GEOJSON_COORDS, new double[] { x, y }) + .endObject(); + try (XContentParser parser = createParser(pointInJson)) { + parser.nextToken(); + Point point = PARSER_WITHOUT_GEOHASH.parseObject(parser, true, GeoUtils.EffectivePoint.BOTTOM_LEFT); + assertEquals(x, point.getX(), 0.001); + assertEquals(y, point.getY(), 0.001); + } + } + + public void testParseFromArrayOne() throws IOException { + double x = 41.12; + XContentBuilder pointArray = XContentFactory.jsonBuilder().startArray().value(x).endArray(); + try (XContentParser parser = createParser(pointArray)) { + parser.nextToken(); + // parser -> "[41.12]" + Point point = PARSER.parseFromArray(parser, true, FN_IN_ERR_MSG); + assertTrue("Expected an exception", false); + } catch (OpenSearchParseException e) { + // Expected exception + } + } + + public void testParseFromArrayTwo() throws IOException { + double x = 41.12; + double y = -71.34; + XContentBuilder pointArray = XContentFactory.jsonBuilder().startArray().value(x).value(y).endArray(); + try (XContentParser parser = createParser(pointArray)) { + parser.nextToken(); + // parser -> "[41.12, -71.34]" + Point point = PARSER.parseFromArray(parser, true, FN_IN_ERR_MSG); + assertEquals(x, point.getX(), 0.001); + assertEquals(y, point.getY(), 0.001); + } + } + + public void testParseFromArrayThree() throws IOException { + double x = 41.12; + double y = -71.34; + double z = 12.12; + XContentBuilder pointArray = XContentFactory.jsonBuilder().startArray().value(x).value(y).value(z).endArray(); + try (XContentParser parser = createParser(pointArray)) { + parser.nextToken(); + // parser -> "[41.12, -71.34, 12.12]" + Point point = PARSER.parseFromArray(parser, false, FN_IN_ERR_MSG); + assertTrue("Expected an exception", false); + } catch (OpenSearchParseException e) { + // Expected exception + } + } + + public void testParseFromArrayThreeIgnore() throws IOException { + double x = 41.12; + double y = -71.34; + double z = 12.12; + XContentBuilder pointArray = XContentFactory.jsonBuilder().startArray().value(x).value(y).value(z).endArray(); + try (XContentParser parser = createParser(pointArray)) { + parser.nextToken(); + // parser -> "[41.12, -71.34, 12.12]" + Point point = PARSER.parseFromArray(parser, true, FN_IN_ERR_MSG); + assertEquals(x, point.getX(), 0.001); + assertEquals(y, point.getY(), 0.001); + } + } + + public void testParseFromArrayFourIgnore() throws IOException { + double x = 41.12; + double y = -71.34; + double z = 12.12; + double a = 33.12; + XContentBuilder pointArray = XContentFactory.jsonBuilder().startArray().value(x).value(y).value(z).value(a).endArray(); + try (XContentParser parser = createParser(pointArray)) { + parser.nextToken(); + // parser -> "[41.12, -71.34, 12.12, 33.12]" + Point point = PARSER.parseFromArray(parser, false, FN_IN_ERR_MSG); + assertTrue("Expected an exception", false); + } catch (OpenSearchParseException e) { + // Expected exception + } + } + + // "41.12, -71.34]" + public void testParseFromArrayPartialForm() throws IOException { + double x = 41.12; + double y = -71.34; + XContentBuilder pointArray = XContentFactory.jsonBuilder().startArray().value(x).value(y).endArray(); + try (XContentParser parser = createParser(pointArray)) { + parser.nextToken(); + parser.nextToken(); + // parser -> "41.12, -71.34]" + Point point = PARSER.parseFromArray(parser, true, FN_IN_ERR_MSG); + assertEquals(x, point.getX(), 0.001); + assertEquals(y, point.getY(), 0.001); + } + } +}