Skip to content

Commit

Permalink
Add null_value support to geo_point type (#29451)
Browse files Browse the repository at this point in the history 
Adds support for null_value attribute to the geo_point types.

Closes #12998
  • Loading branch information
@imotov
imotov committed Apr 17, 2018
1 parent 3367948 commit 983d6c1
Show file tree
Hide file tree
Showing 6 changed files with 180 additions and 6 deletions.
5 changes: 5 additions & 0 deletions docs/reference/mapping/types/geo-point.asciidoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,11 @@ The following parameters are accepted by `geo_point` fields:
ignored. If `false`, geo-points containing any more than latitude and longitude
(two dimensions) values throw an exception and reject the whole document.

<<null-value,`null_value`>>::

Accepts an geopoint value which is substituted for any explicit `null` values.
Defaults to `null`, which means the field is treated as missing.

==== Using geo-points in scripts

When accessing the value of a geo-point in a script, the value is returned as
Expand Down
36 changes: 36 additions & 0 deletions server/src/main/java/org/elasticsearch/common/geo/GeoUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,14 @@
import org.apache.lucene.spatial.prefix.tree.QuadPrefixTree;
import org.apache.lucene.util.SloppyMath;
import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.unit.DistanceUnit;
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.XContentParser.Token;
import org.elasticsearch.common.xcontent.json.JsonXContent;
import org.elasticsearch.common.xcontent.support.XContentMapValues;
import org.elasticsearch.index.fielddata.FieldData;
import org.elasticsearch.index.fielddata.GeoPointValues;
Expand All @@ -36,6 +41,7 @@
import org.elasticsearch.index.fielddata.SortingNumericDoubleValues;

import java.io.IOException;
import java.io.InputStream;

public class GeoUtils {

Expand Down Expand Up @@ -351,6 +357,36 @@ public static GeoPoint parseGeoPoint(XContentParser parser, GeoPoint point) thro
return parseGeoPoint(parser, point, false);
}

/**
* Parses the value as a geopoint. The following types of values are supported:
* <p>
* Object: has to contain either lat and lon or geohash fields
* <p>
* String: expected to be in "latitude, longitude" format or a geohash
* <p>
* Array: two or more elements, the first element is longitude, the second is latitude, the rest is ignored if ignoreZValue is true
*/
public static GeoPoint parseGeoPoint(Object value, final boolean ignoreZValue) throws ElasticsearchParseException {
try {
XContentBuilder content = JsonXContent.contentBuilder();
content.startObject();
content.field("null_value", value);
content.endObject();

try (InputStream stream = BytesReference.bytes(content).streamInput();
XContentParser parser = JsonXContent.jsonXContent.createParser(
NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE, stream)) {
parser.nextToken(); // start object
parser.nextToken(); // field name
parser.nextToken(); // field value
return parseGeoPoint(parser, new GeoPoint(), ignoreZValue);
}

} catch (IOException ex) {
throw new ElasticsearchParseException("error parsing geopoint", ex);
}
}

/**
* Parse a {@link GeoPoint} with a {@link XContentParser}. A geopoint has one of the following forms:
*
Expand Down
37 changes: 34 additions & 3 deletions server/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ public class GeoPointFieldMapper extends FieldMapper implements ArrayValueMapper
public static class Names {
public static final String IGNORE_MALFORMED = "ignore_malformed";
public static final ParseField IGNORE_Z_VALUE = new ParseField("ignore_z_value");
public static final String NULL_VALUE = "null_value";
}

public static class Defaults {
Expand Down Expand Up @@ -134,7 +135,7 @@ public Mapper.Builder parse(String name, Map<String, Object> node, ParserContext
throws MapperParsingException {
Builder builder = new GeoPointFieldMapper.Builder(name);
parseField(builder, name, node, parserContext);

Object nullValue = null;
for (Iterator<Map.Entry<String, Object>> iterator = node.entrySet().iterator(); iterator.hasNext();) {
Map.Entry<String, Object> entry = iterator.next();
String propName = entry.getKey();
Expand All @@ -147,9 +148,31 @@ public Mapper.Builder parse(String name, Map<String, Object> node, ParserContext
builder.ignoreZValue(XContentMapValues.nodeBooleanValue(propNode,
name + "." + Names.IGNORE_Z_VALUE.getPreferredName()));
iterator.remove();
} else if (propName.equals(Names.NULL_VALUE)) {
if (propNode == null) {
throw new MapperParsingException("Property [null_value] cannot be null.");
}
nullValue = propNode;
iterator.remove();
}
}

if (nullValue != null) {
boolean ignoreZValue = builder.ignoreZValue == null ? Defaults.IGNORE_Z_VALUE.value() : builder.ignoreZValue;
boolean ignoreMalformed = builder.ignoreMalformed == null ? Defaults.IGNORE_MALFORMED.value() : builder.ignoreZValue;
GeoPoint point = GeoUtils.parseGeoPoint(nullValue, ignoreZValue);
if (ignoreMalformed == false) {
if (point.lat() > 90.0 || point.lat() < -90.0) {
throw new IllegalArgumentException("illegal latitude value [" + point.lat() + "]");
}
if (point.lon() > 180.0 || point.lon() < -180) {
throw new IllegalArgumentException("illegal longitude value [" + point.lon() + "]");
}
} else {
GeoUtils.normalizePoint(point);
}
builder.nullValue(point);
}
return builder;
}
}
Expand Down Expand Up @@ -318,7 +341,11 @@ public Mapper parse(ParseContext context) throws IOException {
}
} else if (token == XContentParser.Token.VALUE_STRING) {
parse(context, sparse.resetFromString(context.parser().text(), ignoreZValue.value()));
} else if (token != XContentParser.Token.VALUE_NULL) {
} else if (token == XContentParser.Token.VALUE_NULL) {
if (fieldType.nullValue() != null) {
parse(context, (GeoPoint) fieldType.nullValue());
}
} else {
try {
parse(context, GeoUtils.parseGeoPoint(context.parser(), sparse));
} catch (ElasticsearchParseException e) {
Expand All @@ -337,11 +364,15 @@ public Mapper parse(ParseContext context) throws IOException {
protected void doXContentBody(XContentBuilder builder, boolean includeDefaults, Params params) throws IOException {
super.doXContentBody(builder, includeDefaults, params);
if (includeDefaults || ignoreMalformed.explicit()) {
builder.field(GeoPointFieldMapper.Names.IGNORE_MALFORMED, ignoreMalformed.value());
builder.field(Names.IGNORE_MALFORMED, ignoreMalformed.value());
}
if (includeDefaults || ignoreZValue.explicit()) {
builder.field(Names.IGNORE_Z_VALUE.getPreferredName(), ignoreZValue.value());
}

if (includeDefaults || fieldType().nullValue() != null) {
builder.field(Names.NULL_VALUE, fieldType().nullValue());
}
}

public Explicit<Boolean> ignoreZValue() {
Expand Down
49 changes: 49 additions & 0 deletions server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldMapperTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/
package org.elasticsearch.index.mapper;

import org.apache.lucene.util.BytesRef;
import org.elasticsearch.action.admin.indices.create.CreateIndexRequestBuilder;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.common.Priority;
Expand All @@ -41,10 +42,12 @@
import static org.elasticsearch.common.geo.GeoHashUtils.stringEncode;
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.elasticsearch.index.mapper.GeoPointFieldMapper.Names.IGNORE_Z_VALUE;
import static org.elasticsearch.index.mapper.GeoPointFieldMapper.Names.NULL_VALUE;
import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.notNullValue;

public class GeoPointFieldMapperTests extends ESSingleNodeTestCase {
Expand Down Expand Up @@ -349,4 +352,50 @@ public void testEmptyName() throws Exception {
);
assertThat(e.getMessage(), containsString("name cannot be empty string"));
}

public void testNullValue() throws Exception {
String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("properties").startObject("location")
.field("type", "geo_point")
.field(NULL_VALUE, "1,2")
.endObject().endObject()
.endObject().endObject());

DocumentMapper defaultMapper = createIndex("test").mapperService().documentMapperParser()
.parse("type", new CompressedXContent(mapping));
FieldMapper fieldMapper = defaultMapper.mappers().getMapper("location");
assertThat(fieldMapper, instanceOf(GeoPointFieldMapper.class));

Object nullValue = fieldMapper.fieldType().nullValue();
assertThat(nullValue, equalTo(new GeoPoint(1, 2)));

ParsedDocument doc = defaultMapper.parse(SourceToParse.source("test", "type", "1", BytesReference
.bytes(XContentFactory.jsonBuilder()
.startObject()
.nullField("location")
.endObject()),
XContentType.JSON));

assertThat(doc.rootDoc().getField("location"), notNullValue());
BytesRef defaultValue = doc.rootDoc().getField("location").binaryValue();

doc = defaultMapper.parse(SourceToParse.source("test", "type", "1", BytesReference
.bytes(XContentFactory.jsonBuilder()
.startObject()
.field("location", "1, 2")
.endObject()),
XContentType.JSON));
// Shouldn't matter if we specify the value explicitly or use null value
assertThat(defaultValue, equalTo(doc.rootDoc().getField("location").binaryValue()));

doc = defaultMapper.parse(SourceToParse.source("test", "type", "1", BytesReference
.bytes(XContentFactory.jsonBuilder()
.startObject()
.field("location", "3, 4")
.endObject()),
XContentType.JSON));
// Shouldn't matter if we specify the value explicitly or use null value
assertThat(defaultValue, not(equalTo(doc.rootDoc().getField("location").binaryValue())));
}

}
2 changes: 1 addition & 1 deletion server/src/test/java/org/elasticsearch/index/mapper/NullValueTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
public class NullValueTests extends ESSingleNodeTestCase {
public void testNullNullValue() throws Exception {
IndexService indexService = createIndex("test", Settings.builder().build());
String[] typesToTest = {"integer", "long", "double", "float", "short", "date", "ip", "keyword", "boolean", "byte"};
String[] typesToTest = {"integer", "long", "double", "float", "short", "date", "ip", "keyword", "boolean", "byte", "geo_point"};

for (String type : typesToTest) {
String mapping = Strings.toString(XContentFactory.jsonBuilder()
Expand Down
57 changes: 55 additions & 2 deletions server/src/test/java/org/elasticsearch/index/search/geo/GeoPointParsingTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,26 @@ public void testGeoPointParsing() throws IOException {
GeoPoint point = GeoUtils.parseGeoPoint(objectLatLon(randomPt.lat(), randomPt.lon()));
assertPointsEqual(point, randomPt);

GeoUtils.parseGeoPoint(toObject(objectLatLon(randomPt.lat(), randomPt.lon())), randomBoolean());
assertPointsEqual(point, randomPt);

GeoUtils.parseGeoPoint(arrayLatLon(randomPt.lat(), randomPt.lon()), point);
assertPointsEqual(point, randomPt);

GeoUtils.parseGeoPoint(toObject(arrayLatLon(randomPt.lat(), randomPt.lon())), randomBoolean());
assertPointsEqual(point, randomPt);

GeoUtils.parseGeoPoint(geohash(randomPt.lat(), randomPt.lon()), point);
assertCloseTo(point, randomPt.lat(), randomPt.lon());

GeoUtils.parseGeoPoint(toObject(geohash(randomPt.lat(), randomPt.lon())), randomBoolean());
assertCloseTo(point, randomPt.lat(), randomPt.lon());

GeoUtils.parseGeoPoint(stringLatLon(randomPt.lat(), randomPt.lon()), point);
assertCloseTo(point, randomPt.lat(), randomPt.lon());

GeoUtils.parseGeoPoint(toObject(stringLatLon(randomPt.lat(), randomPt.lon())), randomBoolean());
assertCloseTo(point, randomPt.lat(), randomPt.lon());
}

// Based on #5390
Expand All @@ -99,6 +111,12 @@ public void testInvalidPointEmbeddedObject() throws IOException {
parser.nextToken();
Exception e = expectThrows(ElasticsearchParseException.class, () -> GeoUtils.parseGeoPoint(parser));
assertThat(e.getMessage(), is("field must be either [lat], [lon] or [geohash]"));

XContentParser parser2 = createParser(JsonXContent.jsonXContent, BytesReference.bytes(content));
parser2.nextToken();
e = expectThrows(ElasticsearchParseException.class, () ->
GeoUtils.parseGeoPoint(toObject(parser2), randomBoolean()));
assertThat(e.getMessage(), is("field must be either [lat], [lon] or [geohash]"));
}

public void testInvalidPointLatHashMix() throws IOException {
Expand All @@ -109,9 +127,14 @@ public void testInvalidPointLatHashMix() throws IOException {

XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(content));
parser.nextToken();

Exception e = expectThrows(ElasticsearchParseException.class, () -> GeoUtils.parseGeoPoint(parser));
assertThat(e.getMessage(), is("field must be either lat/lon or geohash"));

XContentParser parser2 = createParser(JsonXContent.jsonXContent, BytesReference.bytes(content));
parser2.nextToken();
e = expectThrows(ElasticsearchParseException.class, () ->
GeoUtils.parseGeoPoint(toObject(parser2), randomBoolean()));
assertThat(e.getMessage(), is("field must be either lat/lon or geohash"));
}

public void testInvalidPointLonHashMix() throws IOException {
Expand All @@ -125,6 +148,12 @@ public void testInvalidPointLonHashMix() throws IOException {

Exception e = expectThrows(ElasticsearchParseException.class, () -> GeoUtils.parseGeoPoint(parser));
assertThat(e.getMessage(), is("field must be either lat/lon or geohash"));

XContentParser parser2 = createParser(JsonXContent.jsonXContent, BytesReference.bytes(content));
parser2.nextToken();
e = expectThrows(ElasticsearchParseException.class, () ->
GeoUtils.parseGeoPoint(toObject(parser2), randomBoolean()));
assertThat(e.getMessage(), is("field must be either lat/lon or geohash"));
}

public void testInvalidField() throws IOException {
Expand All @@ -135,9 +164,15 @@ public void testInvalidField() throws IOException {

XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(content));
parser.nextToken();

Exception e = expectThrows(ElasticsearchParseException.class, () -> GeoUtils.parseGeoPoint(parser));
assertThat(e.getMessage(), is("field must be either [lat], [lon] or [geohash]"));


XContentParser parser2 = createParser(JsonXContent.jsonXContent, BytesReference.bytes(content));
parser2.nextToken();
e = expectThrows(ElasticsearchParseException.class, () ->
GeoUtils.parseGeoPoint(toObject(parser2), randomBoolean()));
assertThat(e.getMessage(), is("field must be either [lat], [lon] or [geohash]"));
}

private XContentParser objectLatLon(double lat, double lon) throws IOException {
Expand Down Expand Up @@ -183,4 +218,22 @@ public static void assertCloseTo(final GeoPoint point, final double lat, final d
assertEquals(point.lat(), lat, TOLERANCE);
assertEquals(point.lon(), lon, TOLERANCE);
}

public static Object toObject(XContentParser parser) throws IOException {
XContentParser.Token token = parser.currentToken();
if (token == XContentParser.Token.VALUE_NULL) {
return null;
} else if (token == XContentParser.Token.VALUE_STRING) {
return parser.text();
} else if (token == XContentParser.Token.VALUE_NUMBER) {
return parser.numberValue();
} else if (token == XContentParser.Token.START_OBJECT) {
return parser.map();
} else if (token == XContentParser.Token.START_ARRAY) {
return parser.list();
} else {
fail("Unexpected token " + token);
}
return null;
}
}

0 comments on commit 983d6c1

Please sign in to comment.