honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Introduces GeoValidationMethod to GeoDistanceSortBuilder 

Previously like in other geo related query parsers we were using
a combination of two booleans for coerce and ignore_malformed
which was error prone and not very clear.

Switched to using GeoValidationMethod instead as we already do
e.g. in GeoBoundingBoxQueryBuilder.

Left support for both, coerce and ignore_malformed in the parser
but deprecated the two in favour of validation method.

Introduced the same deprecation in geo bounding box query builder.
This commit is contained in:
Isabel Drost-Fromm 2016-04-28 12:10:56 +02:00
parent de43bda3f3
commit 78ff4f52d6
4 changed files with 114 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
core/src/main/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilder.java
View File

@ -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");

102
core/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java
View File

@ -47,6 +47,7 @@ import org.elasticsearch.index.fielddata.MultiGeoPointValues;
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;
@ -65,16 +66,18 @@ import java.util.Objects;
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<>();
@ -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.
@ -144,8 +145,7 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
this.sortMode = original.sortMode;
this.nestedFilter = original.nestedFilter;
this.nestedPath = original.nestedPath;
this.coerce = original.coerce;
this.ignoreMalformed = original.ignoreMalformed;
this.validation = original.validation;
}
/**
@ -161,8 +161,7 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
sortMode = in.readOptionalWriteable(SortMode::readFromStream);
nestedFilter = in.readOptionalNamedWriteable(QueryBuilder.class);
nestedPath = in.readOptionalString();
coerce = in.readBoolean();
ignoreMalformed =in.readBoolean();
validation = GeoValidationMethod.readFromStream(in);
}
@Override
@ -175,8 +174,7 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
out.writeOptionalWriteable(sortMode);
out.writeOptionalNamedWriteable(nestedFilter);
out.writeOptionalString(nestedPath);
out.writeBoolean(coerce);
out.writeBoolean(ignoreMalformed);
validation.writeTo(out);
}
/**
@ -257,6 +255,21 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
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
@ -309,26 +322,6 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
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();
@ -354,8 +347,7 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
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();
@ -386,14 +378,14 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
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);
}
/**
@ -417,8 +409,9 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
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();
@ -463,6 +456,8 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
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)) {
@ -498,8 +493,13 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
}
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;
}
@ -512,7 +512,7 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
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(
@ -529,9 +529,9 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
}
}
if (coerce) {
if (GeoValidationMethod.isCoerce(validation)) {
for (GeoPoint point : localPoints) {
GeoUtils.normalizePoint(point, coerce, coerce);
GeoUtils.normalizePoint(point, true, true);
}
}

3
core/src/test/java/org/elasticsearch/search/sort/GeoDistanceSortBuilderIT.java
View File

@ -28,6 +28,7 @@ import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.text.Text;
import org.elasticsearch.common.unit.DistanceUnit;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.index.query.GeoValidationMethod;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.search.builder.SearchSourceBuilder;
import org.elasticsearch.test.ESIntegTestCase;
@ -314,7 +315,7 @@ public class GeoDistanceSortBuilderIT extends ESIntegTestCase {
.setSource(
new SearchSourceBuilder().sort(SortBuilders.geoDistanceSort(LOCATION_FIELD, 2.0, 2.0)
.unit(DistanceUnit.KILOMETERS).geoDistance(GeoDistance.PLANE)
.ignoreMalformed(true).coerce(true))).execute().actionGet();
.validation(GeoValidationMethod.COERCE))).execute().actionGet();
checkCorrectSortOrderForGeoSort(searchResponse);
}

74
core/src/test/java/org/elasticsearch/search/sort/GeoDistanceSortBuilderTests.java
View File

@ -33,6 +33,7 @@ import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.index.mapper.geo.GeoPointFieldMapper;
import org.elasticsearch.index.query.GeoValidationMethod;
import org.elasticsearch.index.query.QueryParseContext;
import org.elasticsearch.indices.query.IndicesQueriesRegistry;
import org.elasticsearch.test.geo.RandomGeoGenerator;
@ -94,10 +95,7 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase<GeoDistanc
result.setNestedPath(RandomSortDataGenerator.randomAscii(result.getNestedPath()));
}
if (randomBoolean()) {
result.coerce(! result.coerce());
}
if (randomBoolean()) {
result.ignoreMalformed(! result.ignoreMalformed());
result.validation(randomFrom(GeoValidationMethod.values()));
}
return result;
@ -118,6 +116,14 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase<GeoDistanc
return result;
}
private static GeoValidationMethod validation(GeoValidationMethod original) {
GeoValidationMethod result;
do {
result = randomFrom(GeoValidationMethod.values());
} while (result == original);
return result;
}
private static DistanceUnit unit(DistanceUnit original) {
int id = -1;
while (id == -1 || (original != null && original.ordinal() == id)) {
@ -149,7 +155,7 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase<GeoDistanc
@Override
protected GeoDistanceSortBuilder mutate(GeoDistanceSortBuilder original) throws IOException {
GeoDistanceSortBuilder result = new GeoDistanceSortBuilder(original);
int parameter = randomIntBetween(0, 9);
int parameter = randomIntBetween(0, 8);
switch (parameter) {
case 0:
while (Arrays.deepEquals(original.points(), result.points())) {
@ -179,12 +185,7 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase<GeoDistanc
result.setNestedPath(RandomSortDataGenerator.randomAscii(original.getNestedPath()));
break;
case 8:
result.coerce(! original.coerce());
break;
case 9:
// ignore malformed will only be set if coerce is set to true
result.coerce(false);
result.ignoreMalformed(! original.ignoreMalformed());
result.validation(validation(original.validation()));
break;
}
return result;
@ -262,7 +263,7 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase<GeoDistanc
try {
GeoDistanceSortBuilder item = GeoDistanceSortBuilder.fromXContent(context, "");
item.ignoreMalformed(false);
item.validation(GeoValidationMethod.STRICT);
item.build(createMockShardContext());
fail("adding reverse sorting option should fail with an exception");
@ -271,6 +272,48 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase<GeoDistanc
}
}
public void testCoerceIsDeprecated() throws IOException {
String json = "{\n" +
" \"testname\" : [ {\n" +
" \"lat\" : -6.046997540714173,\n" +
" \"lon\" : -51.94128329747579\n" +
" } ],\n" +
" \"unit\" : \"m\",\n" +
" \"distance_type\" : \"sloppy_arc\",\n" +
" \"mode\" : \"SUM\",\n" +
" \"coerce\" : true\n" +
"}";
XContentParser itemParser = XContentHelper.createParser(new BytesArray(json));
itemParser.nextToken();
QueryParseContext context = new QueryParseContext(indicesQueriesRegistry, itemParser, ParseFieldMatcher.STRICT);
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> GeoDistanceSortBuilder.fromXContent(context, ""));
assertTrue(e.getMessage().startsWith("Deprecated field "));
}
public void testIgnoreMalformedIsDeprecated() throws IOException {
String json = "{\n" +
" \"testname\" : [ {\n" +
" \"lat\" : -6.046997540714173,\n" +
" \"lon\" : -51.94128329747579\n" +
" } ],\n" +
" \"unit\" : \"m\",\n" +
" \"distance_type\" : \"sloppy_arc\",\n" +
" \"mode\" : \"SUM\",\n" +
" \"ignore_malformed\" : true\n" +
"}";
XContentParser itemParser = XContentHelper.createParser(new BytesArray(json));
itemParser.nextToken();
QueryParseContext context = new QueryParseContext(indicesQueriesRegistry, itemParser, ParseFieldMatcher.STRICT);
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> GeoDistanceSortBuilder.fromXContent(context, ""));
assertTrue(e.getMessage().startsWith("Deprecated field "));
}
public void testSortModeSumIsRejectedInJSON() throws IOException {
String json = "{\n" +
" \"testname\" : [ {\n" +
@ -279,9 +322,7 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase<GeoDistanc
" } ],\n" +
" \"unit\" : \"m\",\n" +
" \"distance_type\" : \"sloppy_arc\",\n" +
" \"mode\" : \"SUM\",\n" +
" \"coerce\" : false,\n" +
" \"ignore_malformed\" : false\n" +
" \"mode\" : \"SUM\"\n" +
"}";
XContentParser itemParser = XContentHelper.createParser(new BytesArray(json));
itemParser.nextToken();
@ -306,8 +347,7 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase<GeoDistanc
" \"boost\" : 5.711116\n" +
" }\n" +
" },\n" +
" \"coerce\" : false,\n" +
" \"ignore_malformed\" : true\n" +
" \"validation_method\" : \"STRICT\"\n" +
" }";
XContentParser itemParser = XContentHelper.createParser(new BytesArray(json));
itemParser.nextToken();