Skip to content

Commit

Permalink
Geo: add validator that only checks altitude (#43893)
Browse files Browse the repository at this point in the history
By default, we don't check ranges while indexing geo_shapes. As a
result, it is possible to index geoshapes that contain contain
coordinates outside of -90 +90 and -180 +180 ranges. Such geoshapes
will currently break SQL and ML retrieval mechanism. This commit removes
these restriction from the validator is used in SQL and ML retrieval.
  • Loading branch information
imotov authored Jul 10, 2019
1 parent 4ad081b commit 6bd1853
Show file tree
Hide file tree
Showing 18 changed files with 248 additions and 11 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
/*
* 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.geo.utils;

import org.elasticsearch.geo.geometry.Circle;
import org.elasticsearch.geo.geometry.Geometry;
import org.elasticsearch.geo.geometry.GeometryCollection;
import org.elasticsearch.geo.geometry.GeometryVisitor;
import org.elasticsearch.geo.geometry.Line;
import org.elasticsearch.geo.geometry.LinearRing;
import org.elasticsearch.geo.geometry.MultiLine;
import org.elasticsearch.geo.geometry.MultiPoint;
import org.elasticsearch.geo.geometry.MultiPolygon;
import org.elasticsearch.geo.geometry.Point;
import org.elasticsearch.geo.geometry.Polygon;
import org.elasticsearch.geo.geometry.Rectangle;

/**
* Validator that only checks that altitude only shows up if ignoreZValue is set to true.
*/
public class StandardValidator implements GeometryValidator {

private final boolean ignoreZValue;

public StandardValidator(boolean ignoreZValue) {
this.ignoreZValue = ignoreZValue;
}

protected void checkAltitude(double zValue) {
if (ignoreZValue == false && Double.isNaN(zValue) == false) {
throw new IllegalArgumentException("found Z value [" + zValue + "] but [ignore_z_value] "
+ "parameter is [" + ignoreZValue + "]");
}
}

@Override
public void validate(Geometry geometry) {
if (ignoreZValue == false) {
geometry.visit(new GeometryVisitor<Void, RuntimeException>() {

@Override
public Void visit(Circle circle) throws RuntimeException {
checkAltitude(circle.getAlt());
return null;
}

@Override
public Void visit(GeometryCollection<?> collection) throws RuntimeException {
for (Geometry g : collection) {
g.visit(this);
}
return null;
}

@Override
public Void visit(Line line) throws RuntimeException {
for (int i = 0; i < line.length(); i++) {
checkAltitude(line.getAlt(i));
}
return null;
}

@Override
public Void visit(LinearRing ring) throws RuntimeException {
for (int i = 0; i < ring.length(); i++) {
checkAltitude(ring.getAlt(i));
}
return null;
}

@Override
public Void visit(MultiLine multiLine) throws RuntimeException {
return visit((GeometryCollection<?>) multiLine);
}

@Override
public Void visit(MultiPoint multiPoint) throws RuntimeException {
return visit((GeometryCollection<?>) multiPoint);
}

@Override
public Void visit(MultiPolygon multiPolygon) throws RuntimeException {
return visit((GeometryCollection<?>) multiPolygon);
}

@Override
public Void visit(Point point) throws RuntimeException {
checkAltitude(point.getAlt());
return null;
}

@Override
public Void visit(Polygon polygon) throws RuntimeException {
polygon.getPolygon().visit(this);
for (int i = 0; i < polygon.getNumberOfHoles(); i++) {
polygon.getHole(i).visit(this);
}
return null;
}

@Override
public Void visit(Rectangle rectangle) throws RuntimeException {
checkAltitude(rectangle.getMinAlt());
checkAltitude(rectangle.getMaxAlt());
return null;
}
});
}
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import org.elasticsearch.geo.utils.GeographyValidator;
import org.elasticsearch.geo.utils.GeometryValidator;
import org.elasticsearch.geo.utils.StandardValidator;
import org.elasticsearch.geo.utils.WellKnownText;

import java.io.IOException;
Expand Down Expand Up @@ -59,5 +60,10 @@ public void testInitValidation() {

ex = expectThrows(IllegalArgumentException.class, () -> validator.validate(new Circle(10, 200, 1)));
assertEquals("invalid longitude 200.0; must be between -180.0 and 180.0", ex.getMessage());

ex = expectThrows(IllegalArgumentException.class, () -> new StandardValidator(false).validate(new Circle(10, 200, 1, 20)));
assertEquals("found Z value [1.0] but [ignore_z_value] parameter is [false]", ex.getMessage());

new StandardValidator(true).validate(new Circle(10, 200, 1, 20));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package org.elasticsearch.geo.geometry;

import org.elasticsearch.geo.utils.GeographyValidator;
import org.elasticsearch.geo.utils.StandardValidator;
import org.elasticsearch.geo.utils.WellKnownText;

import java.io.IOException;
Expand Down Expand Up @@ -58,5 +59,11 @@ public void testInitValidation() {
ex = expectThrows(IllegalArgumentException.class, () -> new GeometryCollection<>(
Arrays.asList(new Point(10, 20), new Point(10, 20, 30))));
assertEquals("all elements of the collection should have the same number of dimension", ex.getMessage());

ex = expectThrows(IllegalArgumentException.class, () -> new StandardValidator(false).validate(
new GeometryCollection<Geometry>(Collections.singletonList(new Point(10, 20, 30)))));
assertEquals("found Z value [30.0] but [ignore_z_value] parameter is [false]", ex.getMessage());

new StandardValidator(true).validate(new GeometryCollection<Geometry>(Collections.singletonList(new Point(10, 20, 30))));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import org.elasticsearch.geo.utils.GeographyValidator;
import org.elasticsearch.geo.utils.GeometryValidator;
import org.elasticsearch.geo.utils.StandardValidator;
import org.elasticsearch.geo.utils.WellKnownText;

import java.io.IOException;
Expand Down Expand Up @@ -59,6 +60,12 @@ public void testInitValidation() {
ex = expectThrows(IllegalArgumentException.class,
() -> validator.validate(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());

ex = expectThrows(IllegalArgumentException.class, () -> new StandardValidator(false).validate(
new Line(new double[]{1, 2}, new double[]{3, 4}, new double[]{6, 5})));
assertEquals("found Z value [6.0] but [ignore_z_value] parameter is [false]", ex.getMessage());

new StandardValidator(true).validate(new Line(new double[]{1, 2}, new double[]{3, 4}, new double[]{6, 5}));
}

public void testWKTValidation() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import org.elasticsearch.geo.utils.GeographyValidator;
import org.elasticsearch.geo.utils.GeometryValidator;
import org.elasticsearch.geo.utils.StandardValidator;
import org.elasticsearch.geo.utils.WellKnownText;
import org.elasticsearch.test.ESTestCase;

Expand Down Expand Up @@ -58,6 +59,12 @@ public void testInitValidation() {
ex = expectThrows(IllegalArgumentException.class,
() -> validator.validate(new LinearRing(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());

ex = expectThrows(IllegalArgumentException.class, () -> new StandardValidator(false).validate(
new LinearRing(new double[]{1, 2, 3, 1}, new double[]{3, 4, 5, 3}, new double[]{1, 1, 1, 1})));
assertEquals("found Z value [1.0] but [ignore_z_value] parameter is [false]", ex.getMessage());

new StandardValidator(true).validate(new LinearRing(new double[]{1, 2, 3, 1}, new double[]{3, 4, 5, 3}, new double[]{1, 1, 1, 1}));
}

public void testVisitor() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package org.elasticsearch.geo.geometry;

import org.elasticsearch.geo.utils.GeographyValidator;
import org.elasticsearch.geo.utils.StandardValidator;
import org.elasticsearch.geo.utils.WellKnownText;

import java.io.IOException;
Expand Down Expand Up @@ -50,4 +51,13 @@ public void testBasicSerialization() throws IOException, ParseException {
assertEquals("multilinestring EMPTY", wkt.toWKT(MultiLine.EMPTY));
assertEquals(MultiLine.EMPTY, wkt.fromWKT("multilinestring EMPTY)"));
}

public void testValidation() {
IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> new StandardValidator(false).validate(
new MultiLine(Collections.singletonList(new Line(new double[]{1, 2}, new double[]{3, 4}, new double[]{6, 5})))));
assertEquals("found Z value [6.0] but [ignore_z_value] parameter is [false]", ex.getMessage());

new StandardValidator(true).validate(
new MultiLine(Collections.singletonList(new Line(new double[]{1, 2}, new double[]{3, 4}, new double[]{6, 5}))));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package org.elasticsearch.geo.geometry;

import org.elasticsearch.geo.utils.GeographyValidator;
import org.elasticsearch.geo.utils.StandardValidator;
import org.elasticsearch.geo.utils.WellKnownText;

import java.io.IOException;
Expand Down Expand Up @@ -61,4 +62,12 @@ public void testBasicSerialization() throws IOException, ParseException {
assertEquals("multipoint EMPTY", wkt.toWKT(MultiPoint.EMPTY));
assertEquals(MultiPoint.EMPTY, wkt.fromWKT("multipoint EMPTY)"));
}

public void testValidation() {
IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> new StandardValidator(false).validate(
new MultiPoint(Collections.singletonList(new Point(1, 2 ,3)))));
assertEquals("found Z value [3.0] but [ignore_z_value] parameter is [false]", ex.getMessage());

new StandardValidator(true).validate(new MultiPoint(Collections.singletonList(new Point(1, 2 ,3))));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package org.elasticsearch.geo.geometry;

import org.elasticsearch.geo.utils.GeographyValidator;
import org.elasticsearch.geo.utils.StandardValidator;
import org.elasticsearch.geo.utils.WellKnownText;

import java.io.IOException;
Expand Down Expand Up @@ -52,4 +53,16 @@ public void testBasicSerialization() throws IOException, ParseException {
assertEquals("multipolygon EMPTY", wkt.toWKT(MultiPolygon.EMPTY));
assertEquals(MultiPolygon.EMPTY, wkt.fromWKT("multipolygon EMPTY)"));
}

public void testValidation() {
IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> new StandardValidator(false).validate(
new MultiPolygon(Collections.singletonList(
new Polygon(new LinearRing(new double[]{1, 2, 3, 1}, new double[]{3, 4, 5, 3}, new double[]{1, 2, 3, 1}))
))));
assertEquals("found Z value [1.0] but [ignore_z_value] parameter is [false]", ex.getMessage());

new StandardValidator(true).validate(
new MultiPolygon(Collections.singletonList(
new Polygon(new LinearRing(new double[]{1, 2, 3, 1}, new double[]{3, 4, 5, 3}, new double[]{1, 2, 3, 1})))));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import org.elasticsearch.geo.utils.GeographyValidator;
import org.elasticsearch.geo.utils.GeometryValidator;
import org.elasticsearch.geo.utils.StandardValidator;
import org.elasticsearch.geo.utils.WellKnownText;

import java.io.IOException;
Expand Down Expand Up @@ -51,6 +52,11 @@ public void testInitValidation() {

ex = expectThrows(IllegalArgumentException.class, () -> validator.validate(new Point(10, 500)));
assertEquals("invalid longitude 500.0; must be between -180.0 and 180.0", ex.getMessage());

ex = expectThrows(IllegalArgumentException.class, () -> new StandardValidator(false).validate(new Point(1, 2, 3)));
assertEquals("found Z value [3.0] but [ignore_z_value] parameter is [false]", ex.getMessage());

new StandardValidator(true).validate(new Point(1, 2, 3));
}

public void testWKTValidation() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package org.elasticsearch.geo.geometry;

import org.elasticsearch.geo.utils.GeographyValidator;
import org.elasticsearch.geo.utils.StandardValidator;
import org.elasticsearch.geo.utils.WellKnownText;

import java.io.IOException;
Expand Down Expand Up @@ -70,6 +71,13 @@ public void testInitValidation() {
() -> new Polygon(new LinearRing(new double[]{1, 2, 3, 1}, new double[]{3, 4, 5, 3}, new double[]{5, 4, 3, 5}),
Collections.singletonList(new LinearRing(new double[]{1, 2, 3, 1}, new double[]{3, 4, 5, 3}))));
assertEquals("holes must have the same number of dimensions as the polygon", ex.getMessage());

ex = expectThrows(IllegalArgumentException.class, () -> new StandardValidator(false).validate(
new Polygon(new LinearRing(new double[]{1, 2, 3, 1}, new double[]{3, 4, 5, 3}, new double[]{1, 2, 3, 1}))));
assertEquals("found Z value [1.0] but [ignore_z_value] parameter is [false]", ex.getMessage());

new StandardValidator(true).validate(
new Polygon(new LinearRing(new double[]{1, 2, 3, 1}, new double[]{3, 4, 5, 3}, new double[]{1, 2, 3, 1})));
}

public void testWKTValidation() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import org.elasticsearch.geo.utils.GeographyValidator;
import org.elasticsearch.geo.utils.GeometryValidator;
import org.elasticsearch.geo.utils.StandardValidator;
import org.elasticsearch.geo.utils.WellKnownText;

import java.io.IOException;
Expand Down Expand Up @@ -59,5 +60,11 @@ public void testInitValidation() {
ex = expectThrows(IllegalArgumentException.class,
() -> validator.validate(new Rectangle(1, 2, 2, 3, 5, Double.NaN)));
assertEquals("only one altitude value is specified", ex.getMessage());

ex = expectThrows(IllegalArgumentException.class, () -> new StandardValidator(false).validate(
new Rectangle(30, 40, 50, 10, 20, 60)));
assertEquals("found Z value [20.0] but [ignore_z_value] parameter is [false]", ex.getMessage());

new StandardValidator(true).validate(new Rectangle(30, 40, 50, 10, 20, 60));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.geo.geometry.Geometry;
import org.elasticsearch.geo.utils.GeographyValidator;
import org.elasticsearch.geo.utils.StandardValidator;
import org.elasticsearch.geo.utils.GeometryValidator;
import org.elasticsearch.geo.utils.WellKnownText;

Expand All @@ -38,10 +38,9 @@ public final class GeometryParser {

private final GeoJson geoJsonParser;
private final WellKnownText wellKnownTextParser;
private final GeometryValidator validator;

public GeometryParser(boolean rightOrientation, boolean coerce, boolean ignoreZValue) {
validator = new GeographyValidator(ignoreZValue);
GeometryValidator validator = new StandardValidator(ignoreZValue);
geoJsonParser = new GeoJson(rightOrientation, coerce, validator);
wellKnownTextParser = new WellKnownText(coerce, validator);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentParseException;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.geo.geometry.Line;
import org.elasticsearch.geo.geometry.LinearRing;
import org.elasticsearch.geo.geometry.Point;
import org.elasticsearch.geo.geometry.Polygon;
Expand Down Expand Up @@ -114,6 +115,20 @@ public void testWKTParsing() throws Exception {
newGeoJson.endObject();
assertEquals("{\"val\":\"point (100.0 10.0)\"}", Strings.toString(newGeoJson));
}

// Make sure we can parse values outside the normal lat lon boundaries
XContentBuilder lineGeoJson = XContentFactory.jsonBuilder()
.startObject()
.field("foo", "LINESTRING (100 0, 200 10)")
.endObject();

try (XContentParser parser = createParser(lineGeoJson)) {
parser.nextToken(); // Start object
parser.nextToken(); // Field Name
parser.nextToken(); // Field Value
assertEquals(new Line(new double[]{0, 10}, new double[]{100, 200} ),
new GeometryParser(true, randomBoolean(), randomBoolean()).parse(parser));
}
}

public void testNullParsing() throws Exception {
Expand Down
Loading

0 comments on commit 6bd1853

Please sign in to comment.