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 f6b049b46789d..c59a9046e0318 100644 --- a/server/src/main/java/org/opensearch/common/geo/GeoPoint.java +++ b/server/src/main/java/org/opensearch/common/geo/GeoPoint.java @@ -121,7 +121,7 @@ public GeoPoint resetFromCoordinates(String value, final boolean ignoreZValue) { if (vals.length > 3) { throw new OpenSearchParseException( "failed to parse [{}], expected 2 or 3 coordinates " + "but found: [{}]", - vals.length, + value, vals.length ); } 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 6a7a319197daf..8e1f6c7094c8d 100644 --- a/server/src/main/java/org/opensearch/common/geo/GeoUtils.java +++ b/server/src/main/java/org/opensearch/common/geo/GeoUtils.java @@ -43,7 +43,6 @@ 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; @@ -53,6 +52,7 @@ import java.io.IOException; import java.util.Collections; +import java.util.HashMap; /** * Useful geo utilities @@ -60,7 +60,8 @@ * @opensearch.internal */ public class GeoUtils { - + private static final String ERR_MSG_INVALID_TOKEN = "token [{}] not allowed"; + private static final String ERR_MSG_INVALID_FIELDS = "field must be either [lon|lat], [type|coordinates], or [geohash]"; /** Maximum valid latitude in degrees. */ public static final double MAX_LAT = 90.0; /** Minimum valid latitude in degrees. */ @@ -74,6 +75,9 @@ public class GeoUtils { public static final String LONGITUDE = "lon"; public static final String GEOHASH = "geohash"; + public static final String GEOJSON_TYPE = "type"; + public static final String GEOJSON_TYPE_POINT = "Point"; + public static final String GEOJSON_COORDS = "coordinates"; /** Earth ellipsoid major axis defined by WGS 84 in meters */ public static final double EARTH_SEMI_MAJOR_AXIS = 6378137.0; // meters (WGS 84) @@ -95,12 +99,6 @@ public class GeoUtils { /** rounding error for quantized latitude and longitude values */ public static final double TOLERANCE = 1E-6; - public static final PointUtils POINT_PARSER; - - static { - POINT_PARSER = new PointUtils(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) { @@ -465,20 +463,18 @@ public static GeoPoint parseGeoPoint(XContentParser parser, GeoPoint point, fina * @param effectivePoint tells which point to use for GeoHash form * @return new {@link GeoPoint} parsed from the parse */ - public static GeoPoint parseGeoPoint(XContentParser parser, GeoPoint point, final boolean ignoreZValue, EffectivePoint effectivePoint) - throws IOException, OpenSearchParseException { + public static GeoPoint parseGeoPoint( + final XContentParser parser, + final GeoPoint point, + final boolean ignoreZValue, + final EffectivePoint effectivePoint + ) throws IOException, OpenSearchParseException { switch (parser.currentToken()) { case START_OBJECT: - try (XContentSubParser subParser = new XContentSubParser(parser)) { - Point pointInObject = POINT_PARSER.parseObject(subParser, ignoreZValue, effectivePoint); - point.reset(pointInObject.getLat(), pointInObject.getLon()); - } + parseGeoPointObject(parser, point, ignoreZValue, effectivePoint); break; case START_ARRAY: - try (XContentSubParser subParser = new XContentSubParser(parser)) { - Point pointInArray = POINT_PARSER.parseFromArray(subParser, ignoreZValue, "geo_point"); - point.reset(pointInArray.getLat(), pointInArray.getLon()); - } + parseGeoPointArray(parser, point, ignoreZValue); break; case VALUE_STRING: String val = parser.text(); @@ -490,6 +486,166 @@ public static GeoPoint parseGeoPoint(XContentParser parser, GeoPoint point, fina return point; } + private static GeoPoint parseGeoPointObject( + final XContentParser parser, + final GeoPoint point, + final boolean ignoreZValue, + final GeoUtils.EffectivePoint effectivePoint + ) throws IOException { + try (XContentSubParser subParser = new XContentSubParser(parser)) { + if (subParser.nextToken() != XContentParser.Token.FIELD_NAME) { + throw new OpenSearchParseException(ERR_MSG_INVALID_TOKEN, subParser.currentToken()); + } + + String field = subParser.currentName(); + if (LONGITUDE.equals(field) || LATITUDE.equals(field)) { + parseGeoPointObjectBasicFields(subParser, point); + } else if (GEOHASH.equals(field)) { + parseGeoHashFields(subParser, point, effectivePoint); + } else if (GEOJSON_TYPE.equals(field) || GEOJSON_COORDS.equals(field)) { + parseGeoJsonFields(subParser, point, ignoreZValue); + } else { + throw new OpenSearchParseException(ERR_MSG_INVALID_FIELDS); + } + + if (subParser.nextToken() != XContentParser.Token.END_OBJECT) { + throw new OpenSearchParseException(ERR_MSG_INVALID_FIELDS); + } + + return point; + } + } + + private static GeoPoint parseGeoPointObjectBasicFields(final XContentParser parser, final GeoPoint point) throws IOException { + HashMap data = new HashMap<>(); + for (int i = 0; i < 2; i++) { + if (i != 0) { + parser.nextToken(); + } + + if (parser.currentToken() != XContentParser.Token.FIELD_NAME) { + break; + } + + String field = parser.currentName(); + if (!LONGITUDE.equals(field) && !LATITUDE.equals(field)) { + throw new OpenSearchParseException(ERR_MSG_INVALID_FIELDS); + } + switch (parser.nextToken()) { + case VALUE_NUMBER: + case VALUE_STRING: + try { + data.put(field, parser.doubleValue(true)); + } catch (NumberFormatException e) { + throw new OpenSearchParseException("[{}] and [{}] must be valid double values", e, LONGITUDE, LATITUDE); + } + break; + default: + throw new OpenSearchParseException("{} must be a number", field); + } + } + + if (data.get(LONGITUDE) == null) { + throw new OpenSearchParseException("field [{}] missing", LONGITUDE); + } + if (data.get(LATITUDE) == null) { + throw new OpenSearchParseException("field [{}] missing", LATITUDE); + } + + return point.reset(data.get(LATITUDE), data.get(LONGITUDE)); + } + + private static GeoPoint parseGeoHashFields( + final XContentParser parser, + final GeoPoint point, + final GeoUtils.EffectivePoint effectivePoint + ) throws IOException { + if (parser.currentToken() != XContentParser.Token.FIELD_NAME) { + throw new OpenSearchParseException(ERR_MSG_INVALID_TOKEN, parser.currentToken()); + } + + if (!GEOHASH.equals(parser.currentName())) { + throw new OpenSearchParseException(ERR_MSG_INVALID_FIELDS); + } + + if (parser.nextToken() != XContentParser.Token.VALUE_STRING) { + throw new OpenSearchParseException("{} must be a string", GEOHASH); + } + + return point.parseGeoHash(parser.text(), effectivePoint); + } + + private static GeoPoint parseGeoJsonFields(final XContentParser parser, final GeoPoint point, final boolean ignoreZValue) + throws IOException { + boolean hasTypePoint = false; + boolean hasCoordinates = false; + for (int i = 0; i < 2; i++) { + if (i != 0) { + parser.nextToken(); + } + + if (parser.currentToken() != XContentParser.Token.FIELD_NAME) { + if (!hasTypePoint) { + throw new OpenSearchParseException("field [{}] missing", GEOJSON_TYPE); + } + if (!hasCoordinates) { + throw new OpenSearchParseException("field [{}] missing", GEOJSON_COORDS); + } + } + + if (GEOJSON_TYPE.equals(parser.currentName())) { + if (parser.nextToken() != XContentParser.Token.VALUE_STRING) { + throw new OpenSearchParseException("{} must be a string", GEOJSON_TYPE); + } + + if (!GEOJSON_TYPE_POINT.equals(parser.text())) { + throw new OpenSearchParseException("{} must be {}", GEOJSON_TYPE, GEOJSON_TYPE_POINT); + } + hasTypePoint = true; + } else if (GEOJSON_COORDS.equals(parser.currentName())) { + if (parser.nextToken() != XContentParser.Token.START_ARRAY) { + throw new OpenSearchParseException("{} must be an array", GEOJSON_COORDS); + } + parseGeoPointArray(parser, point, ignoreZValue); + hasCoordinates = true; + } else { + throw new OpenSearchParseException(ERR_MSG_INVALID_FIELDS); + } + } + + return point; + } + + private static GeoPoint parseGeoPointArray(final XContentParser parser, final GeoPoint point, final boolean ignoreZValue) + throws IOException { + try (XContentSubParser subParser = new XContentSubParser(parser)) { + double x = Double.NaN; + double y = Double.NaN; + + int element = 0; + while (subParser.nextToken() != XContentParser.Token.END_ARRAY) { + if (parser.currentToken() != XContentParser.Token.VALUE_NUMBER) { + throw new OpenSearchParseException("numeric value expected"); + } + element++; + if (element == 1) { + x = parser.doubleValue(); + } else if (element == 2) { + y = parser.doubleValue(); + } else if (element == 3) { + GeoPoint.assertZValue(ignoreZValue, parser.doubleValue()); + } else { + throw new OpenSearchParseException("[geo_point] field type does not accept more than 3 values"); + } + } + + if (element < 2) { + throw new OpenSearchParseException("[geo_point] field type should have at least two dimensions"); + } + return point.reset(y, x); + } + } + /** * Parse a {@link GeoPoint} from a string. The string must have one of the following forms: * diff --git a/server/src/main/java/org/opensearch/common/geo/PointUtils.java b/server/src/main/java/org/opensearch/common/geo/PointUtils.java deleted file mode 100644 index d5d823af05b1f..0000000000000 --- a/server/src/main/java/org/opensearch/common/geo/PointUtils.java +++ /dev/null @@ -1,227 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -package org.opensearch.common.geo; - -import org.opensearch.OpenSearchParseException; -import org.opensearch.common.xcontent.XContentParser; -import org.opensearch.geometry.Point; - -import java.io.IOException; -import java.util.HashMap; -import java.util.Locale; - -/** - * A utility class with a point parser methods supporting different point representation formats - * - * @opensearch.internal - */ -public class PointUtils { - 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_GEOHASH = "geohash"; - private static final String ERR_MSG_INVALID_TOKEN = "token [{}] not allowed"; - - private String fieldNameX; - private String fieldNameY; - private boolean supportGeoHash; - - private String invalidFieldErrMsg; - - public PointUtils(final String fieldNameX, final String fieldNameY, final boolean supportGeoHash) { - this.fieldNameX = fieldNameX; - this.fieldNameY = fieldNameY; - this.supportGeoHash = supportGeoHash; - this.invalidFieldErrMsg = supportGeoHash - ? String.format( - Locale.ROOT, - "field must be either %s/%s, %s/%s, or %s", - fieldNameX, - fieldNameY, - FN_GEOJSON_TYPE, - FN_GEOJSON_COORDS, - FN_GEOHASH - ) - : String.format( - Locale.ROOT, - "field must be either %s/%s, or %s/%s", - fieldNameX, - fieldNameY, - FN_GEOJSON_TYPE, - FN_GEOJSON_COORDS - ); - } - - public Point parseObject(final XContentParser parser, final boolean ignoreZValue, final GeoUtils.EffectivePoint effectivePoint) - throws IOException { - if (parser.currentToken() != XContentParser.Token.START_OBJECT) { - throw new OpenSearchParseException("object is expected"); - } - - parser.nextToken(); - - if (parser.currentToken() == XContentParser.Token.END_OBJECT) { - throw new OpenSearchParseException(invalidFieldErrMsg); - } - - if (parser.currentToken() != XContentParser.Token.FIELD_NAME) { - throw new OpenSearchParseException(ERR_MSG_INVALID_TOKEN, parser.currentToken()); - } - - Point point = null; - String field = parser.currentName(); - if (fieldNameX.equals(field) || fieldNameY.equals(field)) { - point = parseBasicFields(parser); - } else if (supportGeoHash && FN_GEOHASH.equals(field)) { - point = parseGeoHashFields(parser, effectivePoint); - } else if (FN_GEOJSON_TYPE.equals(field) || FN_GEOJSON_COORDS.equals(field)) { - point = parseGeoJsonFields(parser, ignoreZValue); - } else { - throw new OpenSearchParseException(invalidFieldErrMsg); - } - - if (parser.currentToken() != XContentParser.Token.END_OBJECT) { - throw new OpenSearchParseException(invalidFieldErrMsg); - } - - return point; - } - - private Point parseGeoHashFields(final XContentParser parser, final GeoUtils.EffectivePoint effectivePoint) throws IOException { - if (parser.currentToken() != XContentParser.Token.FIELD_NAME) { - throw new OpenSearchParseException(ERR_MSG_INVALID_TOKEN, parser.currentToken()); - } - - String field = parser.currentName(); - if (!FN_GEOHASH.equals(field)) { - throw new OpenSearchParseException(invalidFieldErrMsg); - } - - parser.nextToken(); - - if (parser.currentToken() != XContentParser.Token.VALUE_STRING) { - throw new OpenSearchParseException("{} must be a string", FN_GEOHASH); - } - - String geoHash = parser.text(); - GeoPoint geoPoint = new GeoPoint(); - geoPoint = geoPoint.parseGeoHash(geoHash, effectivePoint); - - parser.nextToken(); - return new Point(geoPoint.lon(), geoPoint.lat()); - } - - private Point parseBasicFields(final XContentParser parser) throws IOException { - HashMap xy = new HashMap<>(); - for (int i = 0; i < 2; i++) { - if (parser.currentToken() != XContentParser.Token.FIELD_NAME) { - break; - } - - String field = parser.currentName(); - if (!fieldNameX.equals(field) && !fieldNameY.equals(field)) { - throw new OpenSearchParseException(invalidFieldErrMsg); - } - parser.nextToken(); - switch (parser.currentToken()) { - case VALUE_NUMBER: - case VALUE_STRING: - try { - xy.put(field, parser.doubleValue(true)); - } catch (NumberFormatException e) { - throw new OpenSearchParseException("[{}] and [{}] must be valid double values", e, fieldNameX, fieldNameY); - } - break; - default: - throw new OpenSearchParseException("{} must be a number", field); - } - parser.nextToken(); - } - - if (xy.get(fieldNameX) == null) { - throw new OpenSearchParseException("field [{}] missing", fieldNameX); - } - if (xy.get(fieldNameY) == null) { - throw new OpenSearchParseException("field [{}] missing", fieldNameY); - } - - return new Point(xy.get(fieldNameX), xy.get(fieldNameY)); - } - - private Point parseGeoJsonFields(final XContentParser parser, final boolean ignoreZValue) throws IOException { - boolean isTypePoint = false; - Point point = null; - for (int i = 0; i < 2; i++) { - if (parser.currentToken() != XContentParser.Token.FIELD_NAME) { - throw new OpenSearchParseException(ERR_MSG_INVALID_TOKEN, parser.currentToken()); - } - - String field = parser.currentName(); - parser.nextToken(); - if (FN_GEOJSON_TYPE.equals(field)) { - if (parser.currentToken() != XContentParser.Token.VALUE_STRING) { - throw new OpenSearchParseException("{} must be a string", FN_GEOJSON_TYPE); - } - - if (!FV_GEOJSON_TYPE_POINT.equals(parser.text())) { - throw new OpenSearchParseException("{} must be {}", FN_GEOJSON_TYPE, FV_GEOJSON_TYPE_POINT); - } - isTypePoint = true; - } else if (FN_GEOJSON_COORDS.equals(field)) { - if (parser.currentToken() != XContentParser.Token.START_ARRAY) { - throw new OpenSearchParseException("{} must be an array", FN_GEOJSON_COORDS); - } - point = parseFromArray(parser, ignoreZValue, FN_GEOJSON_COORDS); - } else { - throw new OpenSearchParseException(invalidFieldErrMsg); - } - parser.nextToken(); - } - - if (!isTypePoint) { - throw new OpenSearchParseException("field [{}] missing", FN_GEOJSON_TYPE); - } - - if (point == null) { - throw new OpenSearchParseException("field [{}] missing", FN_GEOJSON_COORDS); - } - - return point; - } - - public Point parseFromArray(final XContentParser parser, final boolean ignoreZValue, final String fieldName) throws IOException { - double x = Double.NaN; - double y = Double.NaN; - - if (parser.currentToken() == XContentParser.Token.START_ARRAY) { - parser.nextToken(); - } - - int element = 0; - while (parser.currentToken() != XContentParser.Token.END_ARRAY) { - if (parser.currentToken() == XContentParser.Token.VALUE_NUMBER) { - element++; - if (element == 1) { - x = parser.doubleValue(); - } else if (element == 2) { - y = parser.doubleValue(); - } else if (element == 3) { - GeoPoint.assertZValue(ignoreZValue, parser.doubleValue()); - } else { - throw new OpenSearchParseException("[{}] field type does not accept > 3 dimensions", fieldName); - } - } else { - throw new OpenSearchParseException("numeric value expected"); - } - parser.nextToken(); - } - if (element < 2) { - throw new OpenSearchParseException("[{}] field type should have at least two dimensions", fieldName); - } - return new Point(x, y); - } -} diff --git a/server/src/main/java/org/opensearch/index/mapper/AbstractPointGeometryFieldMapper.java b/server/src/main/java/org/opensearch/index/mapper/AbstractPointGeometryFieldMapper.java index 7db3559357ce6..305351ca63b9a 100644 --- a/server/src/main/java/org/opensearch/index/mapper/AbstractPointGeometryFieldMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/AbstractPointGeometryFieldMapper.java @@ -36,11 +36,14 @@ import org.opensearch.common.CheckedBiFunction; import org.opensearch.common.Explicit; import org.opensearch.common.ParseField; -import org.opensearch.common.geo.GeoPoint; +import org.opensearch.common.bytes.BytesReference; import org.opensearch.common.geo.GeometryFormat; import org.opensearch.common.geo.GeometryParser; +import org.opensearch.common.xcontent.DeprecationHandler; import org.opensearch.common.xcontent.LoggingDeprecationHandler; +import org.opensearch.common.xcontent.NamedXContentRegistry; import org.opensearch.common.xcontent.XContentBuilder; +import org.opensearch.common.xcontent.XContentFactory; import org.opensearch.common.xcontent.XContentParser; import org.opensearch.geometry.Geometry; import org.opensearch.geometry.Point; @@ -57,7 +60,7 @@ import static org.opensearch.index.mapper.TypeParsers.parseField; /** - * Base class for for spatial fields that only support indexing points + * Base class for spatial fields that only support indexing points * * @opensearch.internal */ @@ -284,18 +287,16 @@ public List

parse(XContentParser parser) throws IOException, ParseException { if (parser.currentToken() == XContentParser.Token.START_ARRAY) { parser.nextToken(); if (parser.currentToken() == XContentParser.Token.VALUE_NUMBER) { - double x = parser.doubleValue(); - parser.nextToken(); - double y = parser.doubleValue(); - parser.nextToken(); - if (parser.currentToken() == XContentParser.Token.VALUE_NUMBER) { - GeoPoint.assertZValue(ignoreZValue, parser.doubleValue()); - } else if (parser.currentToken() != XContentParser.Token.END_ARRAY) { - throw new OpenSearchParseException("field type does not accept > 3 dimensions"); + XContentBuilder xContentBuilder = reConstructArrayXContent(parser); + try ( + XContentParser subParser = createParser( + parser.getXContentRegistry(), + parser.getDeprecationHandler(), + xContentBuilder + ); + ) { + return Collections.singletonList(process(objectParser.apply(subParser, pointSupplier.get()))); } - P point = pointSupplier.get(); - point.resetCoords(x, y); - return Collections.singletonList(process(point)); } else { ArrayList

points = new ArrayList<>(); while (parser.currentToken() != XContentParser.Token.END_ARRAY) { @@ -305,18 +306,45 @@ public List

parse(XContentParser parser) throws IOException, ParseException { 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()))); } } + private XContentParser createParser( + NamedXContentRegistry namedXContentRegistry, + DeprecationHandler deprecationHandler, + XContentBuilder xContentBuilder + ) throws IOException { + XContentParser subParser = xContentBuilder.contentType() + .xContent() + .createParser(namedXContentRegistry, deprecationHandler, BytesReference.bytes(xContentBuilder).streamInput()); + subParser.nextToken(); + return subParser; + } + + private XContentBuilder reConstructArrayXContent(XContentParser parser) throws IOException { + XContentBuilder builder = XContentFactory.jsonBuilder().startArray(); + int count = 0; + while (parser.currentToken() != XContentParser.Token.END_ARRAY) { + if (++count > 3) { + break; + } + if (parser.currentToken() != XContentParser.Token.VALUE_NUMBER) { + throw new OpenSearchParseException("numeric value expected"); + } + builder.value(parser.doubleValue()); + parser.nextToken(); + } + builder.endArray(); + return builder; + } + @Override public Object format(List

points, String format) { List result = new ArrayList<>(); diff --git a/server/src/test/java/org/opensearch/common/geo/PointUtilsTests.java b/server/src/test/java/org/opensearch/common/geo/PointUtilsTests.java deleted file mode 100644 index d8102ae485f26..0000000000000 --- a/server/src/test/java/org/opensearch/common/geo/PointUtilsTests.java +++ /dev/null @@ -1,216 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -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; -import java.util.Locale; - -public class PointUtilsTests extends OpenSearchTestCase { - private static final PointUtils PARSER; - private static final PointUtils 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 PointUtils(FIELD_NAME_X, FIELD_NAME_Y, true); - PARSER_WITHOUT_GEOHASH = new PointUtils(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(); - OpenSearchParseException ex = expectThrows( - OpenSearchParseException.class, - () -> PARSER.parseObject(parser, true, GeoUtils.EffectivePoint.BOTTOM_LEFT) - ); - assertEquals("field [y] missing", ex.getMessage()); - } - } - - 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(); - OpenSearchParseException ex = expectThrows( - OpenSearchParseException.class, - () -> PARSER.parseObject(parser, true, GeoUtils.EffectivePoint.BOTTOM_LEFT) - ); - assertEquals("field must be either x/y, type/coordinates, or geohash", ex.getMessage()); - } - } - - 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(); - OpenSearchParseException ex = expectThrows( - OpenSearchParseException.class, - () -> PARSER_WITHOUT_GEOHASH.parseObject(parser, true, GeoUtils.EffectivePoint.BOTTOM_LEFT) - ); - assertEquals("field must be either x/y, or type/coordinates", ex.getMessage()); - } - } - - 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]" - OpenSearchParseException ex = expectThrows( - OpenSearchParseException.class, - () -> PARSER.parseFromArray(parser, true, FN_IN_ERR_MSG) - ); - assertEquals(String.format(Locale.ROOT, "[%s] field type should have at least two dimensions", FN_IN_ERR_MSG), ex.getMessage()); - } - } - - 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]" - OpenSearchParseException ex = expectThrows( - OpenSearchParseException.class, - () -> PARSER.parseFromArray(parser, false, FN_IN_ERR_MSG) - ); - assertEquals( - String.format( - Locale.ROOT, - "Exception parsing coordinates: found Z value [%.2f] but [ignore_z_value] parameter is [false]", - z - ), - ex.getMessage() - ); - } - } - - 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]" - OpenSearchParseException ex = expectThrows( - OpenSearchParseException.class, - () -> PARSER.parseFromArray(parser, true, FN_IN_ERR_MSG) - ); - assertEquals(String.format(Locale.ROOT, "[%s] field type does not accept > 3 dimensions", FN_IN_ERR_MSG), ex.getMessage()); - } - } - - // "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); - } - } -} diff --git a/server/src/test/java/org/opensearch/index/search/geo/GeoPointParsingTests.java b/server/src/test/java/org/opensearch/index/search/geo/GeoPointParsingTests.java index 06f36435608bd..ea8d660b78157 100644 --- a/server/src/test/java/org/opensearch/index/search/geo/GeoPointParsingTests.java +++ b/server/src/test/java/org/opensearch/index/search/geo/GeoPointParsingTests.java @@ -50,6 +50,7 @@ import static org.hamcrest.Matchers.is; public class GeoPointParsingTests extends OpenSearchTestCase { + private static final String ERR_MSG_INVALID_FIELDS = "field must be either [lon|lat], [type|coordinates], or [geohash]"; private static final double TOLERANCE = 1E-5; public void testGeoPointReset() throws IOException { @@ -142,12 +143,12 @@ public void testInvalidPointEmbeddedObject() throws IOException { try (XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(content))) { parser.nextToken(); Exception e = expectThrows(OpenSearchParseException.class, () -> GeoUtils.parseGeoPoint(parser)); - assertThat(e.getMessage(), is("field must be either lon/lat, type/coordinates, or geohash")); + assertThat(e.getMessage(), is(ERR_MSG_INVALID_FIELDS)); } try (XContentParser parser2 = createParser(JsonXContent.jsonXContent, BytesReference.bytes(content))) { parser2.nextToken(); Exception e = expectThrows(OpenSearchParseException.class, () -> GeoUtils.parseGeoPoint(toObject(parser2), randomBoolean())); - assertThat(e.getMessage(), is("field must be either lon/lat, type/coordinates, or geohash")); + assertThat(e.getMessage(), is(ERR_MSG_INVALID_FIELDS)); } } @@ -160,12 +161,12 @@ public void testInvalidPointLatHashMix() throws IOException { try (XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(content))) { parser.nextToken(); Exception e = expectThrows(OpenSearchParseException.class, () -> GeoUtils.parseGeoPoint(parser)); - assertThat(e.getMessage(), is("field must be either lon/lat, type/coordinates, or geohash")); + assertThat(e.getMessage(), is(ERR_MSG_INVALID_FIELDS)); } try (XContentParser parser2 = createParser(JsonXContent.jsonXContent, BytesReference.bytes(content))) { parser2.nextToken(); Exception e = expectThrows(OpenSearchParseException.class, () -> GeoUtils.parseGeoPoint(toObject(parser2), randomBoolean())); - assertThat(e.getMessage(), is("field must be either lon/lat, type/coordinates, or geohash")); + assertThat(e.getMessage(), is(ERR_MSG_INVALID_FIELDS)); } } @@ -179,12 +180,12 @@ public void testInvalidPointLonHashMix() throws IOException { parser.nextToken(); Exception e = expectThrows(OpenSearchParseException.class, () -> GeoUtils.parseGeoPoint(parser)); - assertThat(e.getMessage(), is("field must be either lon/lat, type/coordinates, or geohash")); + assertThat(e.getMessage(), is(ERR_MSG_INVALID_FIELDS)); } try (XContentParser parser2 = createParser(JsonXContent.jsonXContent, BytesReference.bytes(content))) { parser2.nextToken(); Exception e = expectThrows(OpenSearchParseException.class, () -> GeoUtils.parseGeoPoint(toObject(parser2), randomBoolean())); - assertThat(e.getMessage(), is("field must be either lon/lat, type/coordinates, or geohash")); + assertThat(e.getMessage(), is(ERR_MSG_INVALID_FIELDS)); } } @@ -197,13 +198,13 @@ public void testInvalidField() throws IOException { try (XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(content))) { parser.nextToken(); Exception e = expectThrows(OpenSearchParseException.class, () -> GeoUtils.parseGeoPoint(parser)); - assertThat(e.getMessage(), is("field must be either lon/lat, type/coordinates, or geohash")); + assertThat(e.getMessage(), is(ERR_MSG_INVALID_FIELDS)); } try (XContentParser parser2 = createParser(JsonXContent.jsonXContent, BytesReference.bytes(content))) { parser2.nextToken(); Exception e = expectThrows(OpenSearchParseException.class, () -> GeoUtils.parseGeoPoint(toObject(parser2), randomBoolean())); - assertThat(e.getMessage(), is("field must be either lon/lat, type/coordinates, or geohash")); + assertThat(e.getMessage(), is(ERR_MSG_INVALID_FIELDS)); } } diff --git a/server/src/test/java/org/opensearch/index/search/geo/GeoUtilsTests.java b/server/src/test/java/org/opensearch/index/search/geo/GeoUtilsTests.java index e10d8b21b695a..6de3312ec940d 100644 --- a/server/src/test/java/org/opensearch/index/search/geo/GeoUtilsTests.java +++ b/server/src/test/java/org/opensearch/index/search/geo/GeoUtilsTests.java @@ -45,8 +45,10 @@ import org.opensearch.test.OpenSearchTestCase; import org.locationtech.spatial4j.context.SpatialContext; import org.locationtech.spatial4j.distance.DistanceUtils; +import org.opensearch.test.geo.RandomGeoGenerator; import java.io.IOException; +import java.util.Random; import static org.opensearch.common.xcontent.XContentFactory.jsonBuilder; import static org.hamcrest.Matchers.allOf; @@ -59,6 +61,7 @@ import static org.hamcrest.Matchers.not; public class GeoUtilsTests extends OpenSearchTestCase { + private static final String ERR_MSG_INVALID_FIELDS = "field must be either [lon|lat], [type|coordinates], or [geohash]"; private static final char[] BASE_32 = { '0', '1', @@ -626,7 +629,7 @@ public void testParseGeoPointExtraField() throws IOException { try (XContentParser parser = createParser(json)) { parser.nextToken(); Exception e = expectThrows(OpenSearchParseException.class, () -> GeoUtils.parseGeoPoint(parser)); - assertThat(e.getMessage(), is("field must be either lon/lat, type/coordinates, or geohash")); + assertThat(e.getMessage(), is(ERR_MSG_INVALID_FIELDS)); } } @@ -638,7 +641,7 @@ public void testParseGeoPointLonLatGeoHash() throws IOException { try (XContentParser parser = createParser(json)) { parser.nextToken(); Exception e = expectThrows(OpenSearchParseException.class, () -> GeoUtils.parseGeoPoint(parser)); - assertThat(e.getMessage(), containsString("field must be either lon/lat, type/coordinates, or geohash")); + assertThat(e.getMessage(), containsString(ERR_MSG_INVALID_FIELDS)); } } @@ -698,6 +701,63 @@ public void testParseGeoPointInvalidType() throws IOException { } } + public void testParserGeoPointGeoJson() throws IOException { + GeoPoint geoPoint = RandomGeoGenerator.randomPoint(new Random()); + double[] coordinates = { geoPoint.getLon(), geoPoint.getLat() }; + XContentBuilder json = jsonBuilder().startObject().field("type", "Point").array("coordinates", coordinates).endObject(); + try (XContentParser parser = createParser(json)) { + parser.nextToken(); + GeoPoint paredPoint = GeoUtils.parseGeoPoint(parser); + assertEquals(geoPoint, paredPoint); + } + } + + public void testParserGeoPointGeoJsonMissingField() throws IOException { + GeoPoint geoPoint = RandomGeoGenerator.randomPoint(new Random()); + double[] coordinates = { geoPoint.getLon(), geoPoint.getLat() }; + XContentBuilder missingType = jsonBuilder().startObject().array("coordinates", coordinates).endObject(); + expectParseException(missingType, "field [type] missing"); + + XContentBuilder missingCoordinates = jsonBuilder().startObject().field("type", "Point").endObject(); + expectParseException(missingCoordinates, "field [coordinates] missing"); + } + + public void testParserGeoPointGeoJsonUnknownField() throws IOException { + GeoPoint geoPoint = RandomGeoGenerator.randomPoint(new Random()); + double[] coordinates = { geoPoint.getLon(), geoPoint.getLat() }; + XContentBuilder unknownField = jsonBuilder().startObject() + .field("type", "Point") + .array("coordinates", coordinates) + .field("unknown", "value") + .endObject(); + expectParseException(unknownField, "field must be either [lon|lat], [type|coordinates], or [geohash]"); + } + + public void testParserGeoPointGeoJsonInvalidValue() throws IOException { + GeoPoint geoPoint = RandomGeoGenerator.randomPoint(new Random()); + double[] coordinates = { geoPoint.getLon(), geoPoint.getLat() }; + XContentBuilder invalidGeoJsonType = jsonBuilder().startObject() + .field("type", "point") + .array("coordinates", coordinates) + .endObject(); + expectParseException(invalidGeoJsonType, "type must be Point"); + + String[] coordinatesInString = { String.valueOf(geoPoint.getLon()), String.valueOf(geoPoint.getLat()) }; + XContentBuilder invalideCoordinatesType = jsonBuilder().startObject() + .field("type", "Point") + .array("coordinates", coordinatesInString) + .endObject(); + expectParseException(invalideCoordinatesType, "numeric value expected"); + } + + private void expectParseException(XContentBuilder content, String errMsg) throws IOException { + try (XContentParser parser = createParser(content)) { + parser.nextToken(); + OpenSearchParseException ex = expectThrows(OpenSearchParseException.class, () -> GeoUtils.parseGeoPoint(parser)); + assertEquals(errMsg, ex.getMessage()); + } + } + public void testPrefixTreeCellSizes() { assertThat(GeoUtils.EARTH_SEMI_MAJOR_AXIS, equalTo(DistanceUtils.EARTH_EQUATORIAL_RADIUS_KM * 1000)); assertThat(GeoUtils.quadTreeCellWidth(0), lessThanOrEqualTo(GeoUtils.EARTH_EQUATOR));