Skip to content

Commit

Permalink
Geo: refactor geo mapper and query builder (elastic#44884)
Browse files Browse the repository at this point in the history 
Refactors out the indexing and query generation logic out of the
mapper and query builder into a separate unit-testable classes.
  • Loading branch information
@imotov
imotov authored Jul 26, 2019
1 parent a76242d commit f603f06
Show file tree
Hide file tree
Showing 20 changed files with 648 additions and 441 deletions.
12 changes: 6 additions & 6 deletions server/src/main/java/org/elasticsearch/common/geo/parsers/GeoJsonParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
import org.elasticsearch.common.unit.DistanceUnit;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.XContentSubParser;
import org.elasticsearch.index.mapper.BaseGeoShapeFieldMapper;
import org.elasticsearch.index.mapper.AbstractGeometryFieldMapper;
import org.locationtech.jts.geom.Coordinate;

import java.io.IOException;
Expand All @@ -42,21 +42,21 @@
* complies with geojson specification: https://tools.ietf.org/html/rfc7946
*/
abstract class GeoJsonParser {
protected static ShapeBuilder parse(XContentParser parser, BaseGeoShapeFieldMapper shapeMapper)
protected static ShapeBuilder parse(XContentParser parser, AbstractGeometryFieldMapper shapeMapper)
throws IOException {
GeoShapeType shapeType = null;
DistanceUnit.Distance radius = null;
CoordinateNode coordinateNode = null;
GeometryCollectionBuilder geometryCollections = null;

Orientation orientation = (shapeMapper == null)
? BaseGeoShapeFieldMapper.Defaults.ORIENTATION.value()
? AbstractGeometryFieldMapper.Defaults.ORIENTATION.value()
: shapeMapper.orientation();
Explicit<Boolean> coerce = (shapeMapper == null)
? BaseGeoShapeFieldMapper.Defaults.COERCE
? AbstractGeometryFieldMapper.Defaults.COERCE
: shapeMapper.coerce();
Explicit<Boolean> ignoreZValue = (shapeMapper == null)
? BaseGeoShapeFieldMapper.Defaults.IGNORE_Z_VALUE
? AbstractGeometryFieldMapper.Defaults.IGNORE_Z_VALUE
: shapeMapper.ignoreZValue();

String malformedException = null;
Expand Down Expand Up @@ -208,7 +208,7 @@ private static Coordinate parseCoordinate(XContentParser parser, boolean ignoreZ
* @return Geometry[] geometries of the GeometryCollection
* @throws IOException Thrown if an error occurs while reading from the XContentParser
*/
static GeometryCollectionBuilder parseGeometries(XContentParser parser, BaseGeoShapeFieldMapper mapper) throws
static GeometryCollectionBuilder parseGeometries(XContentParser parser, AbstractGeometryFieldMapper mapper) throws
IOException {
if (parser.currentToken() != XContentParser.Token.START_ARRAY) {
throw new ElasticsearchParseException("geometries must be an array of geojson objects");
Expand Down
12 changes: 6 additions & 6 deletions server/src/main/java/org/elasticsearch/common/geo/parsers/GeoWKTParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
import org.elasticsearch.common.geo.builders.ShapeBuilder;
import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.mapper.BaseGeoShapeFieldMapper;
import org.elasticsearch.index.mapper.AbstractGeometryFieldMapper;
import org.locationtech.jts.geom.Coordinate;

import java.io.IOException;
Expand Down Expand Up @@ -63,7 +63,7 @@ public class GeoWKTParser {
// no instance
private GeoWKTParser() {}

public static ShapeBuilder parse(XContentParser parser, final BaseGeoShapeFieldMapper shapeMapper)
public static ShapeBuilder parse(XContentParser parser, final AbstractGeometryFieldMapper shapeMapper)
throws IOException, ElasticsearchParseException {
return parseExpectedType(parser, null, shapeMapper);
}
Expand All @@ -75,12 +75,12 @@ public static ShapeBuilder parseExpectedType(XContentParser parser, final GeoSha

/** throws an exception if the parsed geometry type does not match the expected shape type */
public static ShapeBuilder parseExpectedType(XContentParser parser, final GeoShapeType shapeType,
final BaseGeoShapeFieldMapper shapeMapper)
final AbstractGeometryFieldMapper shapeMapper)
throws IOException, ElasticsearchParseException {
try (StringReader reader = new StringReader(parser.text())) {
Explicit<Boolean> ignoreZValue = (shapeMapper == null) ? BaseGeoShapeFieldMapper.Defaults.IGNORE_Z_VALUE :
Explicit<Boolean> ignoreZValue = (shapeMapper == null) ? AbstractGeometryFieldMapper.Defaults.IGNORE_Z_VALUE :
shapeMapper.ignoreZValue();
Explicit<Boolean> coerce = (shapeMapper == null) ? BaseGeoShapeFieldMapper.Defaults.COERCE : shapeMapper.coerce();
Explicit<Boolean> coerce = (shapeMapper == null) ? AbstractGeometryFieldMapper.Defaults.COERCE : shapeMapper.coerce();
// setup the tokenizer; configured to read words w/o numbers
StreamTokenizer tokenizer = new StreamTokenizer(reader);
tokenizer.resetSyntax();
Expand Down Expand Up @@ -258,7 +258,7 @@ private static PolygonBuilder parsePolygon(StreamTokenizer stream, final boolean
return null;
}
PolygonBuilder builder = new PolygonBuilder(parseLinearRing(stream, ignoreZValue, coerce),
BaseGeoShapeFieldMapper.Defaults.ORIENTATION.value());
AbstractGeometryFieldMapper.Defaults.ORIENTATION.value());
while (nextCloserOrComma(stream).equals(COMMA)) {
builder.hole(parseLinearRing(stream, ignoreZValue, coerce));
}
Expand Down
4 changes: 2 additions & 2 deletions server/src/main/java/org/elasticsearch/common/geo/parsers/ShapeParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
import org.elasticsearch.common.xcontent.XContent;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.support.MapXContentParser;
import org.elasticsearch.index.mapper.BaseGeoShapeFieldMapper;
import org.elasticsearch.index.mapper.AbstractGeometryFieldMapper;

import java.io.IOException;
import java.util.Collections;
Expand All @@ -50,7 +50,7 @@ public interface ShapeParser {
* if the parsers current token has been <code>null</code>
* @throws IOException if the input could not be read
*/
static ShapeBuilder parse(XContentParser parser, BaseGeoShapeFieldMapper shapeMapper) throws IOException {
static ShapeBuilder parse(XContentParser parser, AbstractGeometryFieldMapper shapeMapper) throws IOException {
if (parser.currentToken() == XContentParser.Token.VALUE_NULL) {
return null;
} if (parser.currentToken() == XContentParser.Token.START_OBJECT) {
Expand Down
132 changes: 105 additions & 27 deletions ...index/mapper/BaseGeoShapeFieldMapper.java → ...x/mapper/AbstractGeometryFieldMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,22 @@
import org.elasticsearch.Version;
import org.elasticsearch.common.Explicit;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.geo.ShapeRelation;
import org.elasticsearch.common.geo.SpatialStrategy;
import org.elasticsearch.common.geo.builders.ShapeBuilder;
import org.elasticsearch.common.geo.builders.ShapeBuilder.Orientation;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.support.XContentMapValues;
import org.elasticsearch.geo.geometry.Geometry;
import org.elasticsearch.index.mapper.LegacyGeoShapeFieldMapper.DeprecatedParameters;
import org.elasticsearch.index.query.QueryShardContext;
import org.elasticsearch.index.query.QueryShardException;

import java.io.IOException;
import java.text.ParseException;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
Expand All @@ -47,8 +52,8 @@
/**
* Base class for {@link GeoShapeFieldMapper} and {@link LegacyGeoShapeFieldMapper}
*/
public abstract class BaseGeoShapeFieldMapper extends FieldMapper {
public static final String CONTENT_TYPE = "geo_shape";
public abstract class AbstractGeometryFieldMapper<Parsed, Processed> extends FieldMapper {


public static class Names {
public static final ParseField ORIENTATION = new ParseField("orientation");
Expand All @@ -62,7 +67,36 @@ public static class Defaults {
public static final Explicit<Boolean> IGNORE_Z_VALUE = new Explicit<>(true, false);
}

public abstract static class Builder<T extends Builder, Y extends BaseGeoShapeFieldMapper>

/**
* Interface representing an preprocessor in geo-shape indexing pipeline
*/
public interface Indexer<Parsed, Processed> {

Processed prepareForIndexing(Parsed geometry);

Class<Processed> processedClass();

}

/**
* interface representing parser in geo shape indexing pipeline
*/
public interface Parser<Parsed> {

Parsed parse(XContentParser parser, AbstractGeometryFieldMapper mapper) throws IOException, ParseException;

}

/**
* interface representing a query builder that generates a query from the given shape
*/
public interface QueryProcessor {

Query process(Geometry shape, String fieldName, SpatialStrategy strategy, ShapeRelation relation, QueryShardContext context);
}

public abstract static class Builder<T extends Builder, Y extends AbstractGeometryFieldMapper>
extends FieldMapper.Builder<T, Y> {
protected Boolean coerce;
protected Boolean ignoreMalformed;
Expand Down Expand Up @@ -152,7 +186,7 @@ protected void setupFieldType(BuilderContext context) {
throw new IllegalArgumentException("name cannot be empty string");
}

BaseGeoShapeFieldType ft = (BaseGeoShapeFieldType)fieldType();
AbstractGeometryFieldType ft = (AbstractGeometryFieldType)fieldType();
ft.setOrientation(orientation().value());
}
}
Expand Down Expand Up @@ -218,26 +252,32 @@ public Mapper.Builder parse(String name, Map<String, Object> node, ParserContext
}
}

public abstract static class BaseGeoShapeFieldType extends MappedFieldType {
public abstract static class AbstractGeometryFieldType<Parsed, Processed> extends MappedFieldType {
protected Orientation orientation = Defaults.ORIENTATION.value();

protected BaseGeoShapeFieldType() {
protected Indexer<Parsed, Processed> geometryIndexer;

protected Parser<Parsed> geometryParser;

protected QueryProcessor geometryQueryBuilder;

protected AbstractGeometryFieldType() {
setIndexOptions(IndexOptions.DOCS);
setTokenized(false);
setStored(false);
setStoreTermVectors(false);
setOmitNorms(true);
}

protected BaseGeoShapeFieldType(BaseGeoShapeFieldType ref) {
protected AbstractGeometryFieldType(AbstractGeometryFieldType ref) {
super(ref);
this.orientation = ref.orientation;
}

@Override
public boolean equals(Object o) {
if (!super.equals(o)) return false;
BaseGeoShapeFieldType that = (BaseGeoShapeFieldType) o;
AbstractGeometryFieldType that = (AbstractGeometryFieldType) o;
return orientation == that.orientation;
}

Expand All @@ -246,16 +286,6 @@ public int hashCode() {
return Objects.hash(super.hashCode(), orientation);
}

@Override
public String typeName() {
return CONTENT_TYPE;
}

@Override
public void checkCompatibility(MappedFieldType fieldType, List<String> conflicts) {
super.checkCompatibility(fieldType, conflicts);
}

public Orientation orientation() { return this.orientation; }

public void setOrientation(Orientation orientation) {
Expand All @@ -272,16 +302,40 @@ public Query existsQuery(QueryShardContext context) {
public Query termQuery(Object value, QueryShardContext context) {
throw new QueryShardException(context, "Geo fields do not support exact searching, use dedicated geo queries instead");
}

public void setGeometryIndexer(Indexer<Parsed, Processed> geometryIndexer) {
this.geometryIndexer = geometryIndexer;
}

protected Indexer<Parsed, Processed> geometryIndexer() {
return geometryIndexer;
}

public void setGeometryParser(Parser<Parsed> geometryParser) {
this.geometryParser = geometryParser;
}

protected Parser<Parsed> geometryParser() {
return geometryParser;
}

public void setGeometryQueryBuilder(QueryProcessor geometryQueryBuilder) {
this.geometryQueryBuilder = geometryQueryBuilder;
}

public QueryProcessor geometryQueryBuilder() {
return geometryQueryBuilder;
}
}

protected Explicit<Boolean> coerce;
protected Explicit<Boolean> ignoreMalformed;
protected Explicit<Boolean> ignoreZValue;

protected BaseGeoShapeFieldMapper(String simpleName, MappedFieldType fieldType, MappedFieldType defaultFieldType,
Explicit<Boolean> ignoreMalformed, Explicit<Boolean> coerce,
Explicit<Boolean> ignoreZValue, Settings indexSettings,
MultiFields multiFields, CopyTo copyTo) {
protected AbstractGeometryFieldMapper(String simpleName, MappedFieldType fieldType, MappedFieldType defaultFieldType,
Explicit<Boolean> ignoreMalformed, Explicit<Boolean> coerce,
Explicit<Boolean> ignoreZValue, Settings indexSettings,
MultiFields multiFields, CopyTo copyTo) {
super(simpleName, fieldType, defaultFieldType, indexSettings, multiFields, copyTo);
this.coerce = coerce;
this.ignoreMalformed = ignoreMalformed;
Expand All @@ -291,7 +345,7 @@ protected BaseGeoShapeFieldMapper(String simpleName, MappedFieldType fieldType,
@Override
protected void doMerge(Mapper mergeWith) {
super.doMerge(mergeWith);
BaseGeoShapeFieldMapper gsfm = (BaseGeoShapeFieldMapper)mergeWith;
AbstractGeometryFieldMapper gsfm = (AbstractGeometryFieldMapper)mergeWith;
if (gsfm.coerce.explicit()) {
this.coerce = gsfm.coerce;
}
Expand All @@ -310,7 +364,7 @@ protected void parseCreateField(ParseContext context, List<IndexableField> field
@Override
protected void doXContentBody(XContentBuilder builder, boolean includeDefaults, Params params) throws IOException {
builder.field("type", contentType());
BaseGeoShapeFieldType ft = (BaseGeoShapeFieldType)fieldType();
AbstractGeometryFieldType ft = (AbstractGeometryFieldType)fieldType();
if (includeDefaults || ft.orientation() != Defaults.ORIENTATION.value()) {
builder.field(Names.ORIENTATION.getPreferredName(), ft.orientation());
}
Expand Down Expand Up @@ -338,11 +392,35 @@ public Explicit<Boolean> ignoreZValue() {
}

public Orientation orientation() {
return ((BaseGeoShapeFieldType)fieldType).orientation();
return ((AbstractGeometryFieldType)fieldType).orientation();
}

protected abstract void indexShape(ParseContext context, Processed shape);

/** parsing logic for geometry indexing */
@Override
protected String contentType() {
return CONTENT_TYPE;
public void parse(ParseContext context) throws IOException {
AbstractGeometryFieldType fieldType = (AbstractGeometryFieldType)fieldType();

@SuppressWarnings("unchecked") Indexer<Parsed, Processed> geometryIndexer = fieldType.geometryIndexer();
@SuppressWarnings("unchecked") Parser<Parsed> geometryParser = fieldType.geometryParser();
try {
Processed shape = context.parseExternalValue(geometryIndexer.processedClass());
if (shape == null) {
Parsed geometry = geometryParser.parse(context.parser(), this);
if (geometry == null) {
return;
}
shape = geometryIndexer.prepareForIndexing(geometry);
}
indexShape(context, shape);
} catch (Exception e) {
if (ignoreMalformed.value() == false) {
throw new MapperParsingException("failed to parse field [{}] of type [{}]", e, fieldType().name(),
fieldType().typeName());
}
context.addIgnoredField(fieldType().name());
}
}

}
Loading

0 comments on commit f603f06

Please sign in to comment.