Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Geo: Add coerce support to libs/geo WKT parser #43273

Merged
merged 3 commits into from
Jun 18, 2019
Merged

Geo: Add coerce support to libs/geo WKT parser #43273

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
20e1bcf
Geo: Add coerce support to libs/geo WKT parser
imotov Jun 14, 2019
dfcf76a
Address review comments
imotov Jun 17, 2019
e4f1fae
Remove trailing .s
imotov Jun 18, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion libs/geo/src/main/java/org/elasticsearch/geo/geometry/GeometryUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
/**
* Geometry-related utility methods
*/
final class GeometryUtils {
public final class GeometryUtils {
/**
* Minimum longitude value.
*/
Expand Down Expand Up @@ -67,4 +67,12 @@ static void checkLongitude(double longitude) {
}
}

public static double checkAltitude(final boolean ignoreZValue, double zValue) {
if (ignoreZValue == false) {
throw new IllegalArgumentException("found Z value [" + zValue + "] but [ignore_z_value] "
+ "parameter is [" + ignoreZValue + "]");
}
return zValue;
}

}
85 changes: 55 additions & 30 deletions libs/geo/src/main/java/org/elasticsearch/geo/utils/WellKnownText.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.elasticsearch.geo.geometry.Circle;
import org.elasticsearch.geo.geometry.Geometry;
import org.elasticsearch.geo.geometry.GeometryCollection;
import org.elasticsearch.geo.geometry.GeometryUtils;
import org.elasticsearch.geo.geometry.GeometryVisitor;
import org.elasticsearch.geo.geometry.Line;
import org.elasticsearch.geo.geometry.LinearRing;
Expand Down Expand Up @@ -52,12 +53,16 @@ public class WellKnownText {
public static final String COMMA = ",";
public static final String NAN = "NaN";

private static final String NUMBER = "<NUMBER>";
private static final String EOF = "END-OF-STREAM";
private static final String EOL = "END-OF-LINE";
private final String NUMBER = "<NUMBER>";
private final String EOF = "END-OF-STREAM";
private final String EOL = "END-OF-LINE";

public WellKnownText() {
private final boolean coerce;
private final boolean ignoreZValue;

public WellKnownText(boolean coerce, boolean ignoreZValue) {
this.coerce = coerce;
this.ignoreZValue = ignoreZValue;
}

public String toWKT(Geometry geometry) {
Expand Down Expand Up @@ -247,7 +252,7 @@ public Geometry fromWKT(String wkt) throws IOException, ParseException {
/**
* parse geometry from the stream tokenizer
*/
private static Geometry parseGeometry(StreamTokenizer stream) throws IOException, ParseException {
private Geometry parseGeometry(StreamTokenizer stream) throws IOException, ParseException {
final String type = nextWord(stream).toLowerCase(Locale.ROOT);
switch (type) {
case "point":
Expand All @@ -272,7 +277,7 @@ private static Geometry parseGeometry(StreamTokenizer stream) throws IOException
throw new IllegalArgumentException("Unknown geometry type: " + type);
}

private static GeometryCollection<Geometry> parseGeometryCollection(StreamTokenizer stream) throws IOException, ParseException {
private GeometryCollection<Geometry> parseGeometryCollection(StreamTokenizer stream) throws IOException, ParseException {
if (nextEmptyOrOpen(stream).equals(EMPTY)) {
return GeometryCollection.EMPTY;
}
Expand All @@ -284,43 +289,43 @@ private static GeometryCollection<Geometry> parseGeometryCollection(StreamTokeni
return new GeometryCollection<>(shapes);
}

private static Point parsePoint(StreamTokenizer stream) throws IOException, ParseException {
private Point parsePoint(StreamTokenizer stream) throws IOException, ParseException {
if (nextEmptyOrOpen(stream).equals(EMPTY)) {
return Point.EMPTY;
}
double lon = nextNumber(stream);
double lat = nextNumber(stream);
Point pt;
if (isNumberNext(stream)) {
pt = new Point(lat, lon, nextNumber(stream));
pt = new Point(lat, lon, GeometryUtils.checkAltitude(ignoreZValue, nextNumber(stream)));
} else {
pt = new Point(lat, lon);
}
nextCloser(stream);
return pt;
}

private static void parseCoordinates(StreamTokenizer stream, ArrayList<Double> lats, ArrayList<Double> lons, ArrayList<Double> alts)
private void parseCoordinates(StreamTokenizer stream, ArrayList<Double> lats, ArrayList<Double> lons, ArrayList<Double> alts)
throws IOException, ParseException {
parseCoordinate(stream, lats, lons, alts);
while (nextCloserOrComma(stream).equals(COMMA)) {
parseCoordinate(stream, lats, lons, alts);
}
}

private static void parseCoordinate(StreamTokenizer stream, ArrayList<Double> lats, ArrayList<Double> lons, ArrayList<Double> alts)
private void parseCoordinate(StreamTokenizer stream, ArrayList<Double> lats, ArrayList<Double> lons, ArrayList<Double> alts)
throws IOException, ParseException {
lons.add(nextNumber(stream));
lats.add(nextNumber(stream));
if (isNumberNext(stream)) {
alts.add(nextNumber(stream));
alts.add(GeometryUtils.checkAltitude(ignoreZValue, nextNumber(stream)));
}
if (alts.isEmpty() == false && alts.size() != lons.size()) {
throw new ParseException("coordinate dimensions do not match: " + tokenString(stream), stream.lineno());
}
}

private static MultiPoint parseMultiPoint(StreamTokenizer stream) throws IOException, ParseException {
private MultiPoint parseMultiPoint(StreamTokenizer stream) throws IOException, ParseException {
String token = nextEmptyOrOpen(stream);
if (token.equals(EMPTY)) {
return MultiPoint.EMPTY;
Expand All @@ -340,7 +345,7 @@ private static MultiPoint parseMultiPoint(StreamTokenizer stream) throws IOExcep
return new MultiPoint(Collections.unmodifiableList(points));
}

private static Line parseLine(StreamTokenizer stream) throws IOException, ParseException {
private Line parseLine(StreamTokenizer stream) throws IOException, ParseException {
String token = nextEmptyOrOpen(stream);
if (token.equals(EMPTY)) {
return Line.EMPTY;
Expand All @@ -356,7 +361,7 @@ private static Line parseLine(StreamTokenizer stream) throws IOException, ParseE
}
}

private static MultiLine parseMultiLine(StreamTokenizer stream) throws IOException, ParseException {
private MultiLine parseMultiLine(StreamTokenizer stream) throws IOException, ParseException {
String token = nextEmptyOrOpen(stream);
if (token.equals(EMPTY)) {
return MultiLine.EMPTY;
Expand All @@ -369,20 +374,21 @@ private static MultiLine parseMultiLine(StreamTokenizer stream) throws IOExcepti
return new MultiLine(Collections.unmodifiableList(lines));
}

private static LinearRing parsePolygonHole(StreamTokenizer stream) throws IOException, ParseException {
private LinearRing parsePolygonHole(StreamTokenizer stream) throws IOException, ParseException {
nextOpener(stream);
ArrayList<Double> lats = new ArrayList<>();
ArrayList<Double> lons = new ArrayList<>();
ArrayList<Double> alts = new ArrayList<>();
parseCoordinates(stream, lats, lons, alts);
closeLinearRingIfCoerced(lats, lons, alts);
if (alts.isEmpty()) {
return new LinearRing(toArray(lats), toArray(lons));
} else {
return new LinearRing(toArray(lats), toArray(lons), toArray(alts));
}
}

private static Polygon parsePolygon(StreamTokenizer stream) throws IOException, ParseException {
private Polygon parsePolygon(StreamTokenizer stream) throws IOException, ParseException {
if (nextEmptyOrOpen(stream).equals(EMPTY)) {
return Polygon.EMPTY;
}
Expand All @@ -395,6 +401,7 @@ private static Polygon parsePolygon(StreamTokenizer stream) throws IOException,
while (nextCloserOrComma(stream).equals(COMMA)) {
holes.add(parsePolygonHole(stream));
}
closeLinearRingIfCoerced(lats, lons, alts);
LinearRing shell;
if (alts.isEmpty()) {
shell = new LinearRing(toArray(lats), toArray(lons));
Expand All @@ -408,7 +415,25 @@ private static Polygon parsePolygon(StreamTokenizer stream) throws IOException,
}
}

private static MultiPolygon parseMultiPolygon(StreamTokenizer stream) throws IOException, ParseException {
/**
* Treats supplied arrays as coordinates of a linear ring. If the ring is not closed and coerce is set to true,
* the first set of coordinates (lat, lon and alt if available) are added to the end of the arrays.s
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, trailing .s

*/
private void closeLinearRingIfCoerced(ArrayList<Double> lats, ArrayList<Double> lons, ArrayList<Double> alts) {
talevy marked this conversation as resolved.
Show resolved Hide resolved
if (coerce && lats.isEmpty() == false && lons.isEmpty() == false) {
int last = lats.size() - 1;
if (!lats.get(0).equals(lats.get(last)) || !lons.get(0).equals(lons.get(last)) ||
(alts.isEmpty() == false && !alts.get(0).equals(alts.get(last)))) {
lons.add(lons.get(0));
lats.add(lats.get(0));
if (alts.isEmpty() == false) {
alts.add(alts.get(0));
}
}
}
}

private MultiPolygon parseMultiPolygon(StreamTokenizer stream) throws IOException, ParseException {
String token = nextEmptyOrOpen(stream);
if (token.equals(EMPTY)) {
return MultiPolygon.EMPTY;
Expand All @@ -421,7 +446,7 @@ private static MultiPolygon parseMultiPolygon(StreamTokenizer stream) throws IOE
return new MultiPolygon(Collections.unmodifiableList(polygons));
}

private static Rectangle parseBBox(StreamTokenizer stream) throws IOException, ParseException {
private Rectangle parseBBox(StreamTokenizer stream) throws IOException, ParseException {
if (nextEmptyOrOpen(stream).equals(EMPTY)) {
return Rectangle.EMPTY;
}
Expand All @@ -438,7 +463,7 @@ private static Rectangle parseBBox(StreamTokenizer stream) throws IOException, P
}


private static Circle parseCircle(StreamTokenizer stream) throws IOException, ParseException {
private Circle parseCircle(StreamTokenizer stream) throws IOException, ParseException {
if (nextEmptyOrOpen(stream).equals(EMPTY)) {
return Circle.EMPTY;
}
Expand All @@ -457,7 +482,7 @@ private static Circle parseCircle(StreamTokenizer stream) throws IOException, Pa
/**
* next word in the stream
*/
private static String nextWord(StreamTokenizer stream) throws ParseException, IOException {
private String nextWord(StreamTokenizer stream) throws ParseException, IOException {
switch (stream.nextToken()) {
case StreamTokenizer.TT_WORD:
final String word = stream.sval;
Expand All @@ -472,7 +497,7 @@ private static String nextWord(StreamTokenizer stream) throws ParseException, IO
throw new ParseException("expected word but found: " + tokenString(stream), stream.lineno());
}

private static double nextNumber(StreamTokenizer stream) throws IOException, ParseException {
private double nextNumber(StreamTokenizer stream) throws IOException, ParseException {
if (stream.nextToken() == StreamTokenizer.TT_WORD) {
if (stream.sval.equalsIgnoreCase(NAN)) {
return Double.NaN;
Expand All @@ -487,7 +512,7 @@ private static double nextNumber(StreamTokenizer stream) throws IOException, Par
throw new ParseException("expected number but found: " + tokenString(stream), stream.lineno());
}

private static String tokenString(StreamTokenizer stream) {
private String tokenString(StreamTokenizer stream) {
switch (stream.ttype) {
case StreamTokenizer.TT_WORD:
return stream.sval;
Expand All @@ -501,13 +526,13 @@ private static String tokenString(StreamTokenizer stream) {
return "'" + (char) stream.ttype + "'";
}

private static boolean isNumberNext(StreamTokenizer stream) throws IOException {
private boolean isNumberNext(StreamTokenizer stream) throws IOException {
final int type = stream.nextToken();
stream.pushBack();
return type == StreamTokenizer.TT_WORD;
}

private static String nextEmptyOrOpen(StreamTokenizer stream) throws IOException, ParseException {
private String nextEmptyOrOpen(StreamTokenizer stream) throws IOException, ParseException {
final String next = nextWord(stream);
if (next.equals(EMPTY) || next.equals(LPAREN)) {
return next;
Expand All @@ -516,28 +541,28 @@ private static String nextEmptyOrOpen(StreamTokenizer stream) throws IOException
+ " but found: " + tokenString(stream), stream.lineno());
}

private static String nextCloser(StreamTokenizer stream) throws IOException, ParseException {
private String nextCloser(StreamTokenizer stream) throws IOException, ParseException {
if (nextWord(stream).equals(RPAREN)) {
return RPAREN;
}
throw new ParseException("expected " + RPAREN + " but found: " + tokenString(stream), stream.lineno());
}

private static String nextComma(StreamTokenizer stream) throws IOException, ParseException {
private String nextComma(StreamTokenizer stream) throws IOException, ParseException {
if (nextWord(stream).equals(COMMA) == true) {
return COMMA;
}
throw new ParseException("expected " + COMMA + " but found: " + tokenString(stream), stream.lineno());
}

private static String nextOpener(StreamTokenizer stream) throws IOException, ParseException {
private String nextOpener(StreamTokenizer stream) throws IOException, ParseException {
if (nextWord(stream).equals(LPAREN)) {
return LPAREN;
}
throw new ParseException("expected " + LPAREN + " but found: " + tokenString(stream), stream.lineno());
}

private static String nextCloserOrComma(StreamTokenizer stream) throws IOException, ParseException {
private String nextCloserOrComma(StreamTokenizer stream) throws IOException, ParseException {
String token = nextWord(stream);
if (token.equals(COMMA) || token.equals(RPAREN)) {
return token;
Expand All @@ -546,7 +571,7 @@ private static String nextCloserOrComma(StreamTokenizer stream) throws IOExcepti
+ " but found: " + tokenString(stream), stream.lineno());
}

public static String getWKTName(Geometry geometry) {
private static String getWKTName(Geometry geometry) {
return geometry.visit(new GeometryVisitor<String, RuntimeException>() {
@Override
public String visit(Circle circle) {
Expand Down Expand Up @@ -600,7 +625,7 @@ public String visit(Rectangle rectangle) {
});
}

private static double[] toArray(ArrayList<Double> doubles) {
private double[] toArray(ArrayList<Double> doubles) {
return doubles.stream().mapToDouble(i -> i).toArray();
}

Expand Down
2 changes: 1 addition & 1 deletion libs/geo/src/test/java/org/elasticsearch/geo/geometry/BaseGeometryTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ protected Writeable.Reader<T> instanceReader() {
@SuppressWarnings("unchecked")
@Override
protected T copyInstance(T instance, Version version) throws IOException {
WellKnownText wkt = new WellKnownText();
WellKnownText wkt = new WellKnownText(true, true);
String text = wkt.toWKT(instance);
try {
return (T) wkt.fromWKT(text);
Expand Down
2 changes: 1 addition & 1 deletion libs/geo/src/test/java/org/elasticsearch/geo/geometry/CircleTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ protected Circle createTestInstance(boolean hasAlt) {
}

public void testBasicSerialization() throws IOException, ParseException {
WellKnownText wkt = new WellKnownText();
WellKnownText wkt = new WellKnownText(true, true);
assertEquals("circle (20.0 10.0 15.0)", wkt.toWKT(new Circle(10, 20, 15)));
assertEquals(new Circle(10, 20, 15), wkt.fromWKT("circle (20.0 10.0 15.0)"));

Expand Down
2 changes: 1 addition & 1 deletion libs/geo/src/test/java/org/elasticsearch/geo/geometry/GeometryCollectionTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ protected GeometryCollection<Geometry> createTestInstance(boolean hasAlt) {


public void testBasicSerialization() throws IOException, ParseException {
WellKnownText wkt = new WellKnownText();
WellKnownText wkt = new WellKnownText(true, true);
assertEquals("geometrycollection (point (20.0 10.0),point EMPTY)",
wkt.toWKT(new GeometryCollection<Geometry>(Arrays.asList(new Point(10, 20), Point.EMPTY))));

Expand Down
8 changes: 7 additions & 1 deletion libs/geo/src/test/java/org/elasticsearch/geo/geometry/LineTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ protected Line createTestInstance(boolean hasAlt) {
}

public void testBasicSerialization() throws IOException, ParseException {
WellKnownText wkt = new WellKnownText();
WellKnownText wkt = new WellKnownText(true, true);
assertEquals("linestring (3.0 1.0, 4.0 2.0)", wkt.toWKT(new Line(new double[]{1, 2}, new double[]{3, 4})));
assertEquals(new Line(new double[]{1, 2}, new double[]{3, 4}), wkt.fromWKT("linestring (3 1, 4 2)"));

Expand All @@ -54,4 +54,10 @@ public void testInitValidation() {
ex = expectThrows(IllegalArgumentException.class, () -> new Line(new double[]{1, 100, 3, 1}, new double[]{3, 4, 5, 3}));
assertEquals("invalid latitude 100.0; must be between -90.0 and 90.0", ex.getMessage());
}

public void testWKTValidation() {
IllegalArgumentException ex = expectThrows(IllegalArgumentException.class,
() -> new WellKnownText(randomBoolean(), false).fromWKT("linestring (3 1 6, 4 2 5)"));
assertEquals("found Z value [6.0] but [ignore_z_value] parameter is [false]", ex.getMessage());
}
}
2 changes: 1 addition & 1 deletion libs/geo/src/test/java/org/elasticsearch/geo/geometry/LinearRingTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public class LinearRingTests extends ESTestCase {

public void testBasicSerialization() {
UnsupportedOperationException ex = expectThrows(UnsupportedOperationException.class,
() -> new WellKnownText().toWKT(new LinearRing(new double[]{1, 2, 3, 1}, new double[]{3, 4, 5, 3})));
() -> new WellKnownText(true, true).toWKT(new LinearRing(new double[]{1, 2, 3, 1}, new double[]{3, 4, 5, 3})));
assertEquals("line ring cannot be serialized using WKT", ex.getMessage());
}

Expand Down
2 changes: 1 addition & 1 deletion libs/geo/src/test/java/org/elasticsearch/geo/geometry/MultiLineTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ protected MultiLine createTestInstance(boolean hasAlt) {
}

public void testBasicSerialization() throws IOException, ParseException {
WellKnownText wkt = new WellKnownText();
WellKnownText wkt = new WellKnownText(true, true);
assertEquals("multilinestring ((3.0 1.0, 4.0 2.0))", wkt.toWKT(
new MultiLine(Collections.singletonList(new Line(new double[]{1, 2}, new double[]{3, 4})))));
assertEquals(new MultiLine(Collections.singletonList(new Line(new double[]{1, 2}, new double[]{3, 4}))),
Expand Down
2 changes: 1 addition & 1 deletion libs/geo/src/test/java/org/elasticsearch/geo/geometry/MultiPointTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ protected MultiPoint createTestInstance(boolean hasAlt) {
}

public void testBasicSerialization() throws IOException, ParseException {
WellKnownText wkt = new WellKnownText();
WellKnownText wkt = new WellKnownText(true, true);
assertEquals("multipoint (2.0 1.0)", wkt.toWKT(
new MultiPoint(Collections.singletonList(new Point(1, 2)))));
assertEquals(new MultiPoint(Collections.singletonList(new Point(1 ,2))),
Expand Down
2 changes: 1 addition & 1 deletion libs/geo/src/test/java/org/elasticsearch/geo/geometry/MultiPolygonTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ protected MultiPolygon createTestInstance(boolean hasAlt) {
}

public void testBasicSerialization() throws IOException, ParseException {
WellKnownText wkt = new WellKnownText();
WellKnownText wkt = new WellKnownText(true, true);
assertEquals("multipolygon (((3.0 1.0, 4.0 2.0, 5.0 3.0, 3.0 1.0)))",
wkt.toWKT(new MultiPolygon(Collections.singletonList(
new Polygon(new LinearRing(new double[]{1, 2, 3, 1}, new double[]{3, 4, 5, 3}))))));
Expand Down
Loading