From 8a7b20597d0ea8c5393d9c483efe280fcd82a403 Mon Sep 17 00:00:00 2001 From: Shay Banon Date: Wed, 17 Aug 2011 05:54:57 +0300 Subject: [PATCH] Geo Type Mapping: Add validation options to validate lat and lon values, closes #1252. --- .../index/mapper/geo/GeoPointFieldMapper.java | 64 ++++++++++- .../geopoint/LatLonMappingGeoPointTests.java | 100 ++++++++++++++++++ 2 files changed, 159 insertions(+), 5 deletions(-) diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/geo/GeoPointFieldMapper.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/geo/GeoPointFieldMapper.java index 9f4732bdac1..e58ebb4e92c 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/geo/GeoPointFieldMapper.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/geo/GeoPointFieldMapper.java @@ -20,6 +20,7 @@ package org.elasticsearch.index.mapper.geo; import org.apache.lucene.document.Field; +import org.elasticsearch.ElasticSearchIllegalArgumentException; import org.elasticsearch.common.Strings; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; @@ -77,6 +78,8 @@ public class GeoPointFieldMapper implements Mapper, ArrayValueMapperParser { public static final boolean ENABLE_LATLON = false; public static final boolean ENABLE_GEOHASH = false; public static final int PRECISION = GeoHashUtils.PRECISION; + public static final boolean VALIDATE_LAT = false; + public static final boolean VALIDATE_LON = false; } public static class Builder extends Mapper.Builder { @@ -93,6 +96,9 @@ public class GeoPointFieldMapper implements Mapper, ArrayValueMapperParser { private Field.Store store = Defaults.STORE; + boolean validateLat = Defaults.VALIDATE_LAT; + boolean validateLon = Defaults.VALIDATE_LON; + public Builder(String name) { super(name); this.builder = this; @@ -158,7 +164,9 @@ public class GeoPointFieldMapper implements Mapper, ArrayValueMapperParser { context.path().pathType(origPathType); - return new GeoPointFieldMapper(name, pathType, enableLatLon, enableGeoHash, precisionStep, precision, latMapper, lonMapper, geohashMapper, geoStringMapper); + return new GeoPointFieldMapper(name, pathType, enableLatLon, enableGeoHash, precisionStep, precision, + latMapper, lonMapper, geohashMapper, geoStringMapper, + validateLon, validateLat); } } @@ -181,6 +189,13 @@ public class GeoPointFieldMapper implements Mapper, ArrayValueMapperParser { builder.precisionStep(XContentMapValues.nodeIntegerValue(fieldNode)); } else if (fieldName.equals("geohash_precision")) { builder.precision(XContentMapValues.nodeIntegerValue(fieldNode)); + } else if (fieldName.equals("validate")) { + builder.validateLat = XContentMapValues.nodeBooleanValue(fieldNode); + builder.validateLon = XContentMapValues.nodeBooleanValue(fieldNode); + } else if (fieldName.equals("validate_lon")) { + builder.validateLon = XContentMapValues.nodeBooleanValue(fieldNode); + } else if (fieldName.equals("validate_lat")) { + builder.validateLat = XContentMapValues.nodeBooleanValue(fieldNode); } } return builder; @@ -207,8 +222,12 @@ public class GeoPointFieldMapper implements Mapper, ArrayValueMapperParser { private final StringFieldMapper geoStringMapper; + private final boolean validateLon; + private final boolean validateLat; + public GeoPointFieldMapper(String name, ContentPath.Type pathType, boolean enableLatLon, boolean enableGeoHash, Integer precisionStep, int precision, - NumberFieldMapper latMapper, NumberFieldMapper lonMapper, StringFieldMapper geohashMapper, StringFieldMapper geoStringMapper) { + NumberFieldMapper latMapper, NumberFieldMapper lonMapper, StringFieldMapper geohashMapper, StringFieldMapper geoStringMapper, + boolean validateLon, boolean validateLat) { this.name = name; this.pathType = pathType; this.enableLatLon = enableLatLon; @@ -220,6 +239,9 @@ public class GeoPointFieldMapper implements Mapper, ArrayValueMapperParser { this.lonMapper = lonMapper; this.geoStringMapper = geoStringMapper; this.geohashMapper = geohashMapper; + + this.validateLat = validateLat; + this.validateLon = validateLon; } @Override public String name() { @@ -324,8 +346,18 @@ public class GeoPointFieldMapper implements Mapper, ArrayValueMapperParser { geohashMapper.parse(context); } if (enableLatLon) { + if (validateLat) { + if (lat > 90.0 || lat < -90.0) { + throw new ElasticSearchIllegalArgumentException("illegal latitude value [" + lat + "] for " + name); + } + } context.externalValue(lat); latMapper.parse(context); + if (validateLon) { + if (lon > 180.0 || lon < -180) { + throw new ElasticSearchIllegalArgumentException("illegal longitude value [" + lon + "] for " + name); + } + } context.externalValue(lon); lonMapper.parse(context); } @@ -333,16 +365,28 @@ public class GeoPointFieldMapper implements Mapper, ArrayValueMapperParser { private void parseGeohash(ParseContext context, String geohash) throws IOException { double[] values = GeoHashUtils.decode(geohash); - context.externalValue(Double.toString(values[0]) + ',' + Double.toString(values[1])); + double lat = values[0]; + double lon = values[1]; + context.externalValue(Double.toString(lat) + ',' + Double.toString(lon)); geoStringMapper.parse(context); if (enableGeoHash) { context.externalValue(geohash); geohashMapper.parse(context); } if (enableLatLon) { - context.externalValue(values[0]); + if (validateLat) { + if (lat > 90.0 || lat < -90.0) { + throw new ElasticSearchIllegalArgumentException("illegal latitude value [" + lat + "] for " + name); + } + } + context.externalValue(lat); latMapper.parse(context); - context.externalValue(values[1]); + if (validateLon) { + if (lon > 180.0 || lon < -180) { + throw new ElasticSearchIllegalArgumentException("illegal longitude value [" + lon + "] for " + name); + } + } + context.externalValue(lon); lonMapper.parse(context); } } @@ -401,6 +445,16 @@ public class GeoPointFieldMapper implements Mapper, ArrayValueMapperParser { if (precisionStep != null) { builder.field("precision_step", precisionStep); } + if (validateLat && validateLon) { + builder.field("validate", true); + } else { + if (validateLat != Defaults.VALIDATE_LAT) { + builder.field("validate_lat", validateLat); + } + if (validateLon != Defaults.VALIDATE_LON) { + builder.field("validate_lon", validateLon); + } + } builder.endObject(); return builder; diff --git a/modules/elasticsearch/src/test/java/org/elasticsearch/index/mapper/geopoint/LatLonMappingGeoPointTests.java b/modules/elasticsearch/src/test/java/org/elasticsearch/index/mapper/geopoint/LatLonMappingGeoPointTests.java index 807c9c2aadc..5cdf4e1f977 100644 --- a/modules/elasticsearch/src/test/java/org/elasticsearch/index/mapper/geopoint/LatLonMappingGeoPointTests.java +++ b/modules/elasticsearch/src/test/java/org/elasticsearch/index/mapper/geopoint/LatLonMappingGeoPointTests.java @@ -19,6 +19,7 @@ package org.elasticsearch.index.mapper.geopoint; +import org.elasticsearch.ElasticSearchIllegalArgumentException; import org.elasticsearch.common.Numbers; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.index.mapper.DocumentMapper; @@ -35,6 +36,105 @@ import static org.hamcrest.Matchers.*; */ public class LatLonMappingGeoPointTests { + @Test public void testValidateLatLonValues() throws Exception { + String mapping = XContentFactory.jsonBuilder().startObject().startObject("type") + .startObject("properties").startObject("point").field("type", "geo_point").field("lat_lon", true).field("validate", true).endObject().endObject() + .endObject().endObject().string(); + + DocumentMapper defaultMapper = MapperTests.newParser().parse(mapping); + + + ParsedDocument doc = defaultMapper.parse("type", "1", XContentFactory.jsonBuilder() + .startObject() + .startObject("point").field("lat", 90).field("lon", 1.3).endObject() + .endObject() + .copiedBytes()); + + try { + defaultMapper.parse("type", "1", XContentFactory.jsonBuilder() + .startObject() + .startObject("point").field("lat", -91).field("lon", 1.3).endObject() + .endObject() + .copiedBytes()); + assert false; + } catch (ElasticSearchIllegalArgumentException e) { + + } + + try { + defaultMapper.parse("type", "1", XContentFactory.jsonBuilder() + .startObject() + .startObject("point").field("lat", 91).field("lon", 1.3).endObject() + .endObject() + .copiedBytes()); + assert false; + } catch (ElasticSearchIllegalArgumentException e) { + + } + + try { + defaultMapper.parse("type", "1", XContentFactory.jsonBuilder() + .startObject() + .startObject("point").field("lat", 1.2).field("lon", -181).endObject() + .endObject() + .copiedBytes()); + assert false; + } catch (ElasticSearchIllegalArgumentException e) { + + } + + try { + defaultMapper.parse("type", "1", XContentFactory.jsonBuilder() + .startObject() + .startObject("point").field("lat", 1.2).field("lon", 181).endObject() + .endObject() + .copiedBytes()); + assert false; + } catch (ElasticSearchIllegalArgumentException e) { + + } + } + + + @Test public void testNoValidateLatLonValues() throws Exception { + String mapping = XContentFactory.jsonBuilder().startObject().startObject("type") + .startObject("properties").startObject("point").field("type", "geo_point").field("lat_lon", true).endObject().endObject() + .endObject().endObject().string(); + + DocumentMapper defaultMapper = MapperTests.newParser().parse(mapping); + + + ParsedDocument doc = defaultMapper.parse("type", "1", XContentFactory.jsonBuilder() + .startObject() + .startObject("point").field("lat", 90).field("lon", 1.3).endObject() + .endObject() + .copiedBytes()); + + defaultMapper.parse("type", "1", XContentFactory.jsonBuilder() + .startObject() + .startObject("point").field("lat", -91).field("lon", 1.3).endObject() + .endObject() + .copiedBytes()); + + defaultMapper.parse("type", "1", XContentFactory.jsonBuilder() + .startObject() + .startObject("point").field("lat", 91).field("lon", 1.3).endObject() + .endObject() + .copiedBytes()); + + defaultMapper.parse("type", "1", XContentFactory.jsonBuilder() + .startObject() + .startObject("point").field("lat", 1.2).field("lon", -181).endObject() + .endObject() + .copiedBytes()); + + defaultMapper.parse("type", "1", XContentFactory.jsonBuilder() + .startObject() + .startObject("point").field("lat", 1.2).field("lon", 181).endObject() + .endObject() + .copiedBytes()); + } + @Test public void testLatLonValues() throws Exception { String mapping = XContentFactory.jsonBuilder().startObject().startObject("type") .startObject("properties").startObject("point").field("type", "geo_point").field("lat_lon", true).endObject().endObject()