Skip to content

Commit

Permalink
Merge pull request elastic#18036 from MaineC/enhancement/switch_geodi…
Browse files Browse the repository at this point in the history
…stancesortbuilder_to_geovalidationmethod

Introduces GeoValidationMethod to GeoDistanceSortBuilder
  • Loading branch information
Isabel Drost-Fromm committed May 4, 2016
2 parents ffcb132 + 283a6d2 commit 247b5c8
Show file tree
Hide file tree
Showing 15 changed files with 297 additions and 107 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,12 @@ public class GeoBoundingBoxQueryBuilder extends AbstractQueryBuilder<GeoBounding
*/
public static final boolean DEFAULT_IGNORE_UNMAPPED = false;

private static final ParseField IGNORE_MALFORMED_FIELD = new ParseField("ignore_malformed");
private static final ParseField TYPE_FIELD = new ParseField("type");
private static final ParseField VALIDATION_METHOD_FIELD = new ParseField("validation_method");
private static final ParseField COERCE_FIELD = new ParseField("coerce", "normalize");
private static final ParseField COERCE_FIELD =new ParseField("coerce", "normalize")
.withAllDeprecated("use field validation_method instead");
private static final ParseField IGNORE_MALFORMED_FIELD = new ParseField("ignore_malformed")
.withAllDeprecated("use field validation_method instead");
private static final ParseField FIELD_FIELD = new ParseField("field");
private static final ParseField TOP_FIELD = new ParseField("top");
private static final ParseField BOTTOM_FIELD = new ParseField("bottom");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,10 @@ public class GeoDistanceQueryBuilder extends AbstractQueryBuilder<GeoDistanceQue
public static final boolean DEFAULT_IGNORE_UNMAPPED = false;

private static final ParseField VALIDATION_METHOD_FIELD = new ParseField("validation_method");
private static final ParseField IGNORE_MALFORMED_FIELD = new ParseField("ignore_malformed");
private static final ParseField COERCE_FIELD = new ParseField("coerce", "normalize");
private static final ParseField IGNORE_MALFORMED_FIELD = new ParseField("ignore_malformed")
.withAllDeprecated("use validation_method instead");
private static final ParseField COERCE_FIELD = new ParseField("coerce", "normalize")
.withAllDeprecated("use validation_method instead");
private static final ParseField OPTIMIZE_BBOX_FIELD = new ParseField("optimize_bbox");
private static final ParseField DISTANCE_TYPE_FIELD = new ParseField("distance_type");
private static final ParseField UNIT_FIELD = new ParseField("unit");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,10 @@ public class GeoDistanceRangeQueryBuilder extends AbstractQueryBuilder<GeoDistan
private static final ParseField NAME_FIELD = new ParseField("_name");
private static final ParseField BOOST_FIELD = new ParseField("boost");
private static final ParseField OPTIMIZE_BBOX_FIELD = new ParseField("optimize_bbox");
private static final ParseField COERCE_FIELD = new ParseField("coerce", "normalize");
private static final ParseField IGNORE_MALFORMED_FIELD = new ParseField("ignore_malformed");
private static final ParseField COERCE_FIELD = new ParseField("coerce", "normalize")
.withAllDeprecated("use validation_method instead");
private static final ParseField IGNORE_MALFORMED_FIELD = new ParseField("ignore_malformed")
.withAllDeprecated("use validation_method instead");
private static final ParseField VALIDATION_METHOD = new ParseField("validation_method");
private static final ParseField IGNORE_UNMAPPED_FIELD = new ParseField("ignore_unmapped");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,10 @@ public class GeoPolygonQueryBuilder extends AbstractQueryBuilder<GeoPolygonQuery
*/
public static final boolean DEFAULT_IGNORE_UNMAPPED = false;

private static final ParseField COERCE_FIELD = new ParseField("coerce", "normalize");
private static final ParseField IGNORE_MALFORMED_FIELD = new ParseField("ignore_malformed");
private static final ParseField COERCE_FIELD = new ParseField("coerce", "normalize")
.withAllDeprecated("use validation_method instead");
private static final ParseField IGNORE_MALFORMED_FIELD = new ParseField("ignore_malformed")
.withAllDeprecated("use validation_method instead");
private static final ParseField VALIDATION_METHOD = new ParseField("validation_method");
private static final ParseField POINTS_FIELD = new ParseField("points");
private static final ParseField IGNORE_UNMAPPED_FIELD = new ParseField("ignore_unmapped");
Expand Down Expand Up @@ -232,9 +234,7 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep
builder.endArray();
builder.endObject();

builder.field(COERCE_FIELD.getPreferredName(), GeoValidationMethod.isCoerce(validationMethod));
builder.field(IGNORE_MALFORMED_FIELD.getPreferredName(),
GeoValidationMethod.isIgnoreMalformed(validationMethod));
builder.field(VALIDATION_METHOD.getPreferredName(), validationMethod);
builder.field(IGNORE_UNMAPPED_FIELD.getPreferredName(), ignoreUnmapped);

printBoostAndQueryName(builder);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import org.elasticsearch.index.fielddata.NumericDoubleValues;
import org.elasticsearch.index.fielddata.SortedNumericDoubleValues;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.index.query.GeoValidationMethod;
import org.elasticsearch.index.query.QueryBuilder;
import org.elasticsearch.index.query.QueryParseContext;
import org.elasticsearch.index.query.QueryShardContext;
Expand All @@ -65,16 +66,18 @@
public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder> {
public static final String NAME = "_geo_distance";
public static final String ALTERNATIVE_NAME = "_geoDistance";
public static final boolean DEFAULT_COERCE = false;
public static final boolean DEFAULT_IGNORE_MALFORMED = false;
public static final ParseField UNIT_FIELD = new ParseField("unit");
public static final ParseField DISTANCE_TYPE_FIELD = new ParseField("distance_type");
public static final ParseField COERCE_FIELD = new ParseField("coerce", "normalize");
public static final ParseField IGNORE_MALFORMED_FIELD = new ParseField("ignore_malformed");
public static final ParseField SORTMODE_FIELD = new ParseField("mode", "sort_mode");
public static final ParseField NESTED_PATH_FIELD = new ParseField("nested_path");
public static final ParseField NESTED_FILTER_FIELD = new ParseField("nested_filter");
public static final ParseField REVERSE_FORBIDDEN = new ParseField("reverse");
public static final GeoValidationMethod DEFAULT_VALIDATION = GeoValidationMethod.DEFAULT;

private static final ParseField UNIT_FIELD = new ParseField("unit");
private static final ParseField DISTANCE_TYPE_FIELD = new ParseField("distance_type");
private static final ParseField VALIDATION_METHOD_FIELD = new ParseField("validation_method");
private static final ParseField IGNORE_MALFORMED_FIELD = new ParseField("ignore_malformed")
.withAllDeprecated("use validation_method instead");
private static final ParseField COERCE_FIELD = new ParseField("coerce", "normalize")
.withAllDeprecated("use validation_method instead");
private static final ParseField SORTMODE_FIELD = new ParseField("mode", "sort_mode");
private static final ParseField NESTED_PATH_FIELD = new ParseField("nested_path");
private static final ParseField NESTED_FILTER_FIELD = new ParseField("nested_filter");

private final String fieldName;
private final List<GeoPoint> points = new ArrayList<>();
Expand All @@ -87,9 +90,7 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
private QueryBuilder nestedFilter;
private String nestedPath;

// TODO switch to GeoValidationMethod enum
private boolean coerce = DEFAULT_COERCE;
private boolean ignoreMalformed = DEFAULT_IGNORE_MALFORMED;
private GeoValidationMethod validation = DEFAULT_VALIDATION;

/**
* Constructs a new distance based sort on a geo point like field.
Expand Down Expand Up @@ -144,8 +145,7 @@ public GeoDistanceSortBuilder(String fieldName, String ... geohashes) {
this.sortMode = original.sortMode;
this.nestedFilter = original.nestedFilter;
this.nestedPath = original.nestedPath;
this.coerce = original.coerce;
this.ignoreMalformed = original.ignoreMalformed;
this.validation = original.validation;
}

/**
Expand All @@ -161,8 +161,7 @@ public GeoDistanceSortBuilder(StreamInput in) throws IOException {
sortMode = in.readOptionalWriteable(SortMode::readFromStream);
nestedFilter = in.readOptionalNamedWriteable(QueryBuilder.class);
nestedPath = in.readOptionalString();
coerce = in.readBoolean();
ignoreMalformed =in.readBoolean();
validation = GeoValidationMethod.readFromStream(in);
}

@Override
Expand All @@ -175,8 +174,7 @@ public void writeTo(StreamOutput out) throws IOException {
out.writeOptionalWriteable(sortMode);
out.writeOptionalNamedWriteable(nestedFilter);
out.writeOptionalString(nestedPath);
out.writeBoolean(coerce);
out.writeBoolean(ignoreMalformed);
validation.writeTo(out);
}

/**
Expand Down Expand Up @@ -257,6 +255,21 @@ public DistanceUnit unit() {
return this.unit;
}

/**
* Sets validation method for this sort builder.
*/
public GeoDistanceSortBuilder validation(GeoValidationMethod method) {
this.validation = method;
return this;
}

/**
* Returns the validation method to use for this sort builder.
*/
public GeoValidationMethod validation() {
return validation;
}

/**
* Defines which distance to use for sorting in the case a document contains multiple geo points.
* Possible values: min and max
Expand Down Expand Up @@ -309,26 +322,6 @@ public String getNestedPath() {
return this.nestedPath;
}

public GeoDistanceSortBuilder coerce(boolean coerce) {
this.coerce = coerce;
return this;
}

public boolean coerce() {
return this.coerce;
}

public GeoDistanceSortBuilder ignoreMalformed(boolean ignoreMalformed) {
if (coerce == false) {
this.ignoreMalformed = ignoreMalformed;
}
return this;
}

public boolean ignoreMalformed() {
return this.ignoreMalformed;
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();
Expand All @@ -354,8 +347,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
if (nestedFilter != null) {
builder.field(NESTED_FILTER_FIELD.getPreferredName(), nestedFilter, params);
}
builder.field(COERCE_FIELD.getPreferredName(), coerce);
builder.field(IGNORE_MALFORMED_FIELD.getPreferredName(), ignoreMalformed);
builder.field(VALIDATION_METHOD_FIELD.getPreferredName(), validation);

builder.endObject();
builder.endObject();
Expand Down Expand Up @@ -386,14 +378,14 @@ public boolean equals(Object object) {
Objects.equals(order, other.order) &&
Objects.equals(nestedFilter, other.nestedFilter) &&
Objects.equals(nestedPath, other.nestedPath) &&
Objects.equals(coerce, other.coerce) &&
Objects.equals(ignoreMalformed, other.ignoreMalformed);
Objects.equals(validation, other.validation);
}

@Override
public int hashCode() {
return Objects.hash(this.fieldName, this.points, this.geoDistance,
this.unit, this.sortMode, this.order, this.nestedFilter, this.nestedPath, this.coerce, this.ignoreMalformed);
this.unit, this.sortMode, this.order, this.nestedFilter,
this.nestedPath, this.validation);
}

/**
Expand All @@ -417,8 +409,9 @@ public static GeoDistanceSortBuilder fromXContent(QueryParseContext context, Str
QueryBuilder<?> nestedFilter = null;
String nestedPath = null;

boolean coerce = GeoDistanceSortBuilder.DEFAULT_COERCE;
boolean ignoreMalformed = GeoDistanceSortBuilder.DEFAULT_IGNORE_MALFORMED;
boolean coerce = GeoValidationMethod.DEFAULT_LENIENT_PARSING;
boolean ignoreMalformed = GeoValidationMethod.DEFAULT_LENIENT_PARSING;
GeoValidationMethod validation = null;

XContentParser.Token token;
String currentName = parser.currentName();
Expand Down Expand Up @@ -463,6 +456,8 @@ public static GeoDistanceSortBuilder fromXContent(QueryParseContext context, Str
if (coerce == false) {
ignoreMalformed = ignore_malformed_value;
}
} else if (parseFieldMatcher.match(currentName, VALIDATION_METHOD_FIELD)) {
validation = GeoValidationMethod.fromString(parser.text());
} else if (parseFieldMatcher.match(currentName, SORTMODE_FIELD)) {
sortMode = SortMode.fromString(parser.text());
} else if (parseFieldMatcher.match(currentName, NESTED_PATH_FIELD)) {
Expand Down Expand Up @@ -498,8 +493,13 @@ public static GeoDistanceSortBuilder fromXContent(QueryParseContext context, Str
}
result.setNestedFilter(nestedFilter);
result.setNestedPath(nestedPath);
result.coerce(coerce);
result.ignoreMalformed(ignoreMalformed);
if (validation == null) {
// looks like either validation was left unset or we are parsing old validation json
result.validation(GeoValidationMethod.infer(coerce, ignoreMalformed));
} else {
// ignore deprecated coerce/ignore_malformed
result.validation(validation);
}
return result;
}

Expand All @@ -512,7 +512,7 @@ public SortField build(QueryShardContext context) throws IOException {
localPoints.add(new GeoPoint(geoPoint));
}

if (!indexCreatedBeforeV2_0 && !ignoreMalformed) {
if (!indexCreatedBeforeV2_0 && !GeoValidationMethod.isIgnoreMalformed(validation)) {
for (GeoPoint point : localPoints) {
if (GeoUtils.isValidLatitude(point.lat()) == false) {
throw new ElasticsearchParseException(
Expand All @@ -529,9 +529,9 @@ public SortField build(QueryShardContext context) throws IOException {
}
}

if (coerce) {
if (GeoValidationMethod.isCoerce(validation)) {
for (GeoPoint point : localPoints) {
GeoUtils.normalizePoint(point, coerce, coerce);
GeoUtils.normalizePoint(point, true, true);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,42 @@ public void testFromJson() throws IOException {
}
}

public void testFromJsonCoerceFails() throws IOException {
String json =
"{\n" +
" \"geo_bounding_box\" : {\n" +
" \"pin.location\" : {\n" +
" \"top_left\" : [ -74.1, 40.73 ],\n" +
" \"bottom_right\" : [ -71.12, 40.01 ]\n" +
" },\n" +
" \"coerce\" : true,\n" +
" \"type\" : \"MEMORY\",\n" +
" \"ignore_unmapped\" : false,\n" +
" \"boost\" : 1.0\n" +
" }\n" +
"}";
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> parseQuery(json));
assertTrue(e.getMessage().startsWith("Deprecated field "));
}

public void testFromJsonIgnoreMalformedFails() throws IOException {
String json =
"{\n" +
" \"geo_bounding_box\" : {\n" +
" \"pin.location\" : {\n" +
" \"top_left\" : [ -74.1, 40.73 ],\n" +
" \"bottom_right\" : [ -71.12, 40.01 ]\n" +
" },\n" +
" \"ignore_malformed\" : true,\n" +
" \"type\" : \"MEMORY\",\n" +
" \"ignore_unmapped\" : false,\n" +
" \"boost\" : 1.0\n" +
" }\n" +
"}";
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> parseQuery(json));
assertTrue(e.getMessage().startsWith("Deprecated field "));
}

@Override
public void testMustRewrite() throws IOException {
assumeTrue("test runs only when at least a type is registered", getCurrentTypes().length > 0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,40 @@ public void testFromJson() throws IOException {
assertEquals(json, 12000.0, parsed.distance(), 0.0001);
}

public void testFromCoerceFails() throws IOException {
String json =
"{\n" +
" \"geo_distance\" : {\n" +
" \"pin.location\" : [ -70.0, 40.0 ],\n" +
" \"distance\" : 12000.0,\n" +
" \"distance_type\" : \"sloppy_arc\",\n" +
" \"optimize_bbox\" : \"memory\",\n" +
" \"coerce\" : true,\n" +
" \"ignore_unmapped\" : false,\n" +
" \"boost\" : 1.0\n" +
" }\n" +
"}";
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> parseQuery(json));
assertTrue(e.getMessage().startsWith("Deprecated field "));
}

public void testFromJsonIgnoreMalformedFails() throws IOException {
String json =
"{\n" +
" \"geo_distance\" : {\n" +
" \"pin.location\" : [ -70.0, 40.0 ],\n" +
" \"distance\" : 12000.0,\n" +
" \"distance_type\" : \"sloppy_arc\",\n" +
" \"optimize_bbox\" : \"memory\",\n" +
" \"ignore_malformed\" : true,\n" +
" \"ignore_unmapped\" : false,\n" +
" \"boost\" : 1.0\n" +
" }\n" +
"}";
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> parseQuery(json));
assertTrue(e.getMessage().startsWith("Deprecated field "));
}

@Override
public void testMustRewrite() throws IOException {
assumeTrue("test runs only when at least a type is registered", getCurrentTypes().length > 0);
Expand Down
Loading

0 comments on commit 247b5c8

Please sign in to comment.