Skip to content

Commit

Permalink
Remove legacy geo code from AbstractGeometryQueryBuilder classes (#74741
Browse files Browse the repository at this point in the history 
) (#74889)

removes references to Legacy ShapeParser and ShapeBuilder in AbstractGeometryQueryBuilder classes
in favour to Geometry and GeometryParser.
  • Loading branch information
@iverase
iverase committed Jul 5, 2021
1 parent f194818 commit 33c2207
Show file tree
Hide file tree
Showing 17 changed files with 188 additions and 178 deletions.
9 changes: 6 additions & 3 deletions ...les/percolator/src/test/java/org/elasticsearch/percolator/PercolateQueryBuilderTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -187,10 +188,12 @@ public void testIndexedDocumentDoesNotExist() throws IOException {
}

@Override
protected Set<String> getObjectsHoldingArbitraryContent() {
protected Map<String, String> getObjectsHoldingArbitraryContent() {
//document contains arbitrary content, no error expected when an object is added to it
return new HashSet<>(Arrays.asList(PercolateQueryBuilder.DOCUMENT_FIELD.getPreferredName(),
PercolateQueryBuilder.DOCUMENTS_FIELD.getPreferredName()));
final Map<String, String> objects = new HashMap<>();
objects.put(PercolateQueryBuilder.DOCUMENT_FIELD.getPreferredName(), null);
objects.put(PercolateQueryBuilder.DOCUMENTS_FIELD.getPreferredName(), null);
return objects;
}

public void testRequiredParameters() {
Expand Down
26 changes: 5 additions & 21 deletions server/src/main/java/org/elasticsearch/index/query/AbstractGeometryQueryBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import org.elasticsearch.common.geo.GeometryIO;
import org.elasticsearch.common.geo.GeometryParser;
import org.elasticsearch.common.geo.ShapeRelation;
import org.elasticsearch.common.geo.builders.ShapeBuilder;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
Expand Down Expand Up @@ -77,21 +76,6 @@ public abstract class AbstractGeometryQueryBuilder<QB extends AbstractGeometryQu

protected boolean ignoreUnmapped = DEFAULT_IGNORE_UNMAPPED;

/**
* Creates a new ShapeQueryBuilder whose Query will be against the given
* field name using the given Shape
*
* @param fieldName
* Name of the field that will be queried
* @param shape
* Shape used in the Query
* @deprecated use {@link #AbstractGeometryQueryBuilder(String, Geometry)} instead
*/
@Deprecated
protected AbstractGeometryQueryBuilder(String fieldName, ShapeBuilder shape) {
this(fieldName, shape == null ? null : shape.buildGeometry(), null, null);
}

/**
* Creates a new AbstractGeometryQueryBuilder whose Query will be against the given
* field name using the given Shape
Expand Down Expand Up @@ -396,19 +380,19 @@ private void fetch(Client client, GetRequest getRequest, String path, ActionList
try {
if (response.isExists() == false) {
throw new IllegalArgumentException("Shape with ID [" + getRequest.id() + "] in type [" + getRequest.type()
+ "] not found");
+ "] not found");
}
if (response.isSourceEmpty()) {
throw new IllegalArgumentException("Shape with ID [" + getRequest.id() + "] in type [" + getRequest.type() +
"] source disabled");
"] source disabled");
}
String[] pathElements = path.split("\\.");
int currentPathSlot = 0;

// It is safe to use EMPTY here because this never uses namedObject
try (XContentParser parser = XContentHelper
.createParser(NamedXContentRegistry.EMPTY,
LoggingDeprecationHandler.INSTANCE, response.getSourceAsBytesRef())) {
.createParser(NamedXContentRegistry.EMPTY,
LoggingDeprecationHandler.INSTANCE, response.getSourceAsBytesRef())) {
XContentParser.Token currentToken;
while ((currentToken = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
if (currentToken == XContentParser.Token.FIELD_NAME) {
Expand Down Expand Up @@ -520,7 +504,7 @@ protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) throws
protected abstract static class ParsedGeometryQueryParams {
public String fieldName;
public ShapeRelation relation;
public ShapeBuilder shape;
public Geometry shape;

public String id = null;
public String type = null;
Expand Down
27 changes: 8 additions & 19 deletions server/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,12 @@

import org.apache.lucene.search.ConstantScoreQuery;
import org.apache.lucene.search.Query;
import org.elasticsearch.common.geo.GeometryParser;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.common.xcontent.ParseField;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.common.geo.ShapeRelation;
import org.elasticsearch.common.geo.SpatialStrategy;
import org.elasticsearch.common.geo.builders.ShapeBuilder;
import org.elasticsearch.common.geo.parsers.ShapeParser;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.logging.DeprecationCategory;
Expand All @@ -28,6 +27,7 @@
import org.elasticsearch.index.mapper.MappedFieldType;

import java.io.IOException;
import java.text.ParseException;
import java.util.Objects;
import java.util.function.Supplier;

Expand Down Expand Up @@ -58,22 +58,6 @@ public GeoShapeQueryBuilder(String fieldName, Geometry shape) {
super(fieldName, shape);
}

/**
* Creates a new GeoShapeQueryBuilder whose Query will be against the given
* field name using the given Shape
*
* @param fieldName
* Name of the field that will be queried
* @param shape
* Shape used in the Query
*
* @deprecated use {@link #GeoShapeQueryBuilder(String, Geometry)} instead
*/
@Deprecated
public GeoShapeQueryBuilder(String fieldName, ShapeBuilder shape) {
super(fieldName, shape);
}

public GeoShapeQueryBuilder(String fieldName, Supplier<Geometry> shapeSupplier, String indexedShapeId,
@Nullable String indexedShapeType) {
super(fieldName, shapeSupplier, indexedShapeId, indexedShapeType);
Expand Down Expand Up @@ -216,12 +200,17 @@ protected GeoShapeQueryBuilder doRewrite(QueryRewriteContext queryRewriteContext

private static class ParsedGeoShapeQueryParams extends ParsedGeometryQueryParams {
SpatialStrategy strategy;
private final GeometryParser geometryParser = new GeometryParser(true, true, true);

@Override
protected boolean parseXContentField(XContentParser parser) throws IOException {
SpatialStrategy strategy;
if (SHAPE_FIELD.match(parser.currentName(), parser.getDeprecationHandler())) {
this.shape = ShapeParser.parse(parser);
try {
this.shape = geometryParser.parse(parser);
} catch (ParseException e) {
throw new IOException(e);
}
return true;
} else if (STRATEGY_FIELD.match(parser.currentName(), parser.getDeprecationHandler())) {
String strategyName = parser.text();
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/org/elasticsearch/index/query/QueryBuilders.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -679,7 +679,7 @@ public static GeoShapeQueryBuilder geoShapeQuery(String name, Geometry shape) th
*/
@Deprecated
public static GeoShapeQueryBuilder geoShapeQuery(String name, ShapeBuilder shape) throws IOException {
return new GeoShapeQueryBuilder(name, shape);
return new GeoShapeQueryBuilder(name, shape.buildGeometry());
}

public static GeoShapeQueryBuilder geoShapeQuery(String name, String indexedShapeId) {
Expand Down
11 changes: 5 additions & 6 deletions server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderGeoPointTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
package org.elasticsearch.index.query;

import org.elasticsearch.common.geo.ShapeRelation;
import org.elasticsearch.common.geo.builders.ShapeBuilder;
import org.elasticsearch.test.geo.RandomShapeGenerator;
import org.elasticsearch.geo.GeometryTestUtils;
import org.elasticsearch.geometry.Geometry;

public class GeoShapeQueryBuilderGeoPointTests extends GeoShapeQueryBuilderTests {

Expand All @@ -18,14 +18,13 @@ protected String fieldName() {
}

protected GeoShapeQueryBuilder doCreateTestQueryBuilder(boolean indexedShape) {
RandomShapeGenerator.ShapeType shapeType = RandomShapeGenerator.ShapeType.POLYGON;
ShapeBuilder<?, ?, ?> shape = RandomShapeGenerator.createShapeWithin(random(), null, shapeType);
Geometry geometry = GeometryTestUtils.randomPolygon(false);
GeoShapeQueryBuilder builder;
clearShapeFields();
if (indexedShape == false) {
builder = new GeoShapeQueryBuilder(fieldName(), shape);
builder = new GeoShapeQueryBuilder(fieldName(), geometry);
} else {
indexedShapeToReturn = shape;
indexedShapeToReturn = geometry;
indexedShapeId = randomAlphaOfLengthBetween(3, 20);
builder = new GeoShapeQueryBuilder(fieldName(), indexedShapeId);
if (randomBoolean()) {
Expand Down
27 changes: 14 additions & 13 deletions server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderGeoShapeTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@

import org.elasticsearch.Version;
import org.elasticsearch.common.geo.ShapeRelation;
import org.elasticsearch.common.geo.builders.ShapeBuilder;
import org.elasticsearch.test.geo.RandomShapeGenerator;
import org.elasticsearch.geo.GeometryTestUtils;
import org.elasticsearch.geometry.Geometry;
import org.elasticsearch.geometry.ShapeType;

public class GeoShapeQueryBuilderGeoShapeTests extends GeoShapeQueryBuilderTests {

Expand All @@ -19,19 +20,19 @@ protected String fieldName() {
}

protected GeoShapeQueryBuilder doCreateTestQueryBuilder(boolean indexedShape) {
RandomShapeGenerator.ShapeType shapeType = randomFrom(
RandomShapeGenerator.ShapeType.POINT,
RandomShapeGenerator.ShapeType.MULTIPOINT,
RandomShapeGenerator.ShapeType.LINESTRING,
RandomShapeGenerator.ShapeType.MULTILINESTRING,
RandomShapeGenerator.ShapeType.POLYGON);
ShapeBuilder<?, ?, ?> shape = RandomShapeGenerator.createShapeWithin(random(), null, shapeType);
ShapeType shapeType = randomFrom(
ShapeType.POINT,
ShapeType.MULTIPOINT,
ShapeType.LINESTRING,
ShapeType.MULTILINESTRING,
ShapeType.POLYGON);
Geometry geometry = GeometryTestUtils.randomGeometry(shapeType, false);
GeoShapeQueryBuilder builder;
clearShapeFields();
if (indexedShape == false) {
builder = new GeoShapeQueryBuilder(fieldName(), shape);
builder = new GeoShapeQueryBuilder(fieldName(), geometry);
} else {
indexedShapeToReturn = shape;
indexedShapeToReturn = geometry;
indexedShapeId = randomAlphaOfLengthBetween(3, 20);
builder = new GeoShapeQueryBuilder(fieldName(), indexedShapeId);
if (randomBoolean()) {
Expand All @@ -50,14 +51,14 @@ protected GeoShapeQueryBuilder doCreateTestQueryBuilder(boolean indexedShape) {
if (randomBoolean()) {
SearchExecutionContext context = createSearchExecutionContext();
if (context.indexVersionCreated().onOrAfter(Version.V_7_5_0)) { // CONTAINS is only supported from version 7.5
if (shapeType == RandomShapeGenerator.ShapeType.LINESTRING || shapeType == RandomShapeGenerator.ShapeType.MULTILINESTRING) {
if (shapeType == ShapeType.LINESTRING || shapeType == ShapeType.MULTILINESTRING) {
builder.relation(randomFrom(ShapeRelation.DISJOINT, ShapeRelation.INTERSECTS, ShapeRelation.CONTAINS));
} else {
builder.relation(randomFrom(ShapeRelation.DISJOINT, ShapeRelation.INTERSECTS,
ShapeRelation.WITHIN, ShapeRelation.CONTAINS));
}
} else {
if (shapeType == RandomShapeGenerator.ShapeType.LINESTRING || shapeType == RandomShapeGenerator.ShapeType.MULTILINESTRING) {
if (shapeType == ShapeType.LINESTRING || shapeType == ShapeType.MULTILINESTRING) {
builder.relation(randomFrom(ShapeRelation.DISJOINT, ShapeRelation.INTERSECTS));
} else {
builder.relation(randomFrom(ShapeRelation.DISJOINT, ShapeRelation.INTERSECTS, ShapeRelation.WITHIN));
Expand Down

0 comments on commit 33c2207

Please sign in to comment.