Skip to content

Commit

Permalink
Remove types from GeoShapeQueryBuilder (#47792)
Browse files Browse the repository at this point in the history 
This commit removes the unused 'shapeType' information from AbstractGeoQueryBuilder
and its implementations.

Related to #41059
  • Loading branch information
@romseygeek
romseygeek authored Oct 23, 2019
1 parent 36b03a2 commit 0ed05a9
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 139 deletions.
90 changes: 26 additions & 64 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,11 +22,11 @@
import org.apache.lucene.search.MatchNoDocsQuery;
import org.apache.lucene.search.Query;
import org.apache.lucene.util.SetOnce;
import org.elasticsearch.Version;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.get.GetRequest;
import org.elasticsearch.action.get.GetResponse;
import org.elasticsearch.client.Client;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.common.geo.GeoJson;
Expand All @@ -43,6 +43,7 @@
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.geometry.Geometry;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.index.mapper.MapperService;

import java.io.IOException;
import java.util.ArrayList;
Expand All @@ -55,9 +56,6 @@
*/
public abstract class AbstractGeometryQueryBuilder<QB extends AbstractGeometryQueryBuilder<QB>> extends AbstractQueryBuilder<QB> {

static final String TYPES_DEPRECATION_MESSAGE = "[types removal] Types are deprecated in [geo_shape] queries. " +
"The type should no longer be specified in the [indexed_shape] section.";

public static final String DEFAULT_SHAPE_INDEX_NAME = "shapes";
public static final String DEFAULT_SHAPE_FIELD_NAME = "shape";
public static final ShapeRelation DEFAULT_SHAPE_RELATION = ShapeRelation.INTERSECTS;
Expand All @@ -72,7 +70,6 @@ public abstract class AbstractGeometryQueryBuilder<QB extends AbstractGeometryQu
protected static final ParseField RELATION_FIELD = new ParseField("relation");
protected static final ParseField INDEXED_SHAPE_FIELD = new ParseField("indexed_shape");
protected static final ParseField SHAPE_ID_FIELD = new ParseField("id");
protected static final ParseField SHAPE_TYPE_FIELD = new ParseField("type");
protected static final ParseField SHAPE_INDEX_FIELD = new ParseField("index");
protected static final ParseField SHAPE_PATH_FIELD = new ParseField("path");
protected static final ParseField SHAPE_ROUTING_FIELD = new ParseField("routing");
Expand All @@ -82,7 +79,6 @@ public abstract class AbstractGeometryQueryBuilder<QB extends AbstractGeometryQu
protected final Supplier<Geometry> supplier;

protected final String indexedShapeId;
protected final String indexedShapeType;

protected Geometry shape;
protected String indexedShapeIndex = DEFAULT_SHAPE_INDEX_NAME;
Expand All @@ -105,7 +101,7 @@ public abstract class AbstractGeometryQueryBuilder<QB extends AbstractGeometryQu
*/
@Deprecated
protected AbstractGeometryQueryBuilder(String fieldName, ShapeBuilder shape) {
this(fieldName, shape == null ? null : shape.buildGeometry(), null, null);
this(fieldName, shape == null ? null : shape.buildGeometry(), null);
}

/**
Expand All @@ -118,7 +114,7 @@ protected AbstractGeometryQueryBuilder(String fieldName, ShapeBuilder shape) {
* Shape used in the Query
*/
public AbstractGeometryQueryBuilder(String fieldName, Geometry shape) {
this(fieldName, shape, null, null);
this(fieldName, shape, null);
}

/**
Expand All @@ -131,28 +127,10 @@ public AbstractGeometryQueryBuilder(String fieldName, Geometry shape) {
* ID of the indexed Shape that will be used in the Query
*/
protected AbstractGeometryQueryBuilder(String fieldName, String indexedShapeId) {
this(fieldName, (Geometry) null, indexedShapeId, null);
this(fieldName, (Geometry) null, indexedShapeId);
}

/**
* Creates a new AbstractGeometryQueryBuilder whose Query will be against the given
* field name and will use the Shape found with the given ID in the given
* type
*
* @param fieldName
* Name of the field that will be filtered
* @param indexedShapeId
* ID of the indexed Shape that will be used in the Query
* @param indexedShapeType
* Index type of the indexed Shapes
* @deprecated use {@link #AbstractGeometryQueryBuilder(String, String)} instead
*/
@Deprecated
protected AbstractGeometryQueryBuilder(String fieldName, String indexedShapeId, String indexedShapeType) {
this(fieldName, (Geometry) null, indexedShapeId, indexedShapeType);
}

protected AbstractGeometryQueryBuilder(String fieldName, Geometry shape, String indexedShapeId, @Nullable String indexedShapeType) {
protected AbstractGeometryQueryBuilder(String fieldName, Geometry shape, String indexedShapeId) {
if (fieldName == null) {
throw new IllegalArgumentException("fieldName is required");
}
Expand All @@ -162,17 +140,20 @@ protected AbstractGeometryQueryBuilder(String fieldName, Geometry shape, String
this.fieldName = fieldName;
this.shape = shape;
this.indexedShapeId = indexedShapeId;
this.indexedShapeType = indexedShapeType;
this.supplier = null;
}

protected AbstractGeometryQueryBuilder(String fieldName, Supplier<Geometry> supplier, String indexedShapeId,
@Nullable String indexedShapeType) {
protected AbstractGeometryQueryBuilder(String fieldName, Supplier<Geometry> supplier, String indexedShapeId) {
if (fieldName == null) {
throw new IllegalArgumentException("fieldName is required");
}
if (supplier == null && indexedShapeId == null) {
throw new IllegalArgumentException("either shape or indexedShapeId is required");
}
this.fieldName = fieldName;
this.shape = null;
this.supplier = supplier;
this.indexedShapeId = indexedShapeId;
this.indexedShapeType = indexedShapeType;
this.indexedShapeId = indexedShapeId;;
}

/**
Expand All @@ -184,11 +165,13 @@ protected AbstractGeometryQueryBuilder(StreamInput in) throws IOException {
if (in.readBoolean()) {
shape = GeometryIO.readGeometry(in);
indexedShapeId = null;
indexedShapeType = null;
} else {
shape = null;
indexedShapeId = in.readOptionalString();
indexedShapeType = in.readOptionalString();
if (in.getVersion().before(Version.V_8_0_0)) {
String type = in.readOptionalString();
assert MapperService.SINGLE_MAPPING_NAME.equals(type) : "Expected type [_doc], got [" + type + "]";
}
indexedShapeIndex = in.readOptionalString();
indexedShapePath = in.readOptionalString();
indexedShapeRouting = in.readOptionalString();
Expand All @@ -210,7 +193,9 @@ protected void doWriteTo(StreamOutput out) throws IOException {
GeometryIO.writeGeometry(out, shape);
} else {
out.writeOptionalString(indexedShapeId);
out.writeOptionalString(indexedShapeType);
if (out.getVersion().before(Version.V_8_0_0)) {
out.writeOptionalString(MapperService.SINGLE_MAPPING_NAME);
}
out.writeOptionalString(indexedShapeIndex);
out.writeOptionalString(indexedShapePath);
out.writeOptionalString(indexedShapeRouting);
Expand Down Expand Up @@ -254,17 +239,6 @@ public String indexedShapeId() {
return indexedShapeId;
}

/**
* @return the document type of the indexed Shape that will be used in the
* Query
*
* @deprecated Types are in the process of being removed.
*/
@Deprecated
public String indexedShapeType() {
return indexedShapeType;
}

/**
* Sets the name of the index where the indexed Shape can be found
*
Expand Down Expand Up @@ -372,9 +346,9 @@ public boolean ignoreUnmapped() {
protected abstract void doShapeQueryXContent(XContentBuilder builder, Params params) throws IOException;
/** creates a new ShapeQueryBuilder from the provided field name and shape builder */
protected abstract AbstractGeometryQueryBuilder<QB> newShapeQueryBuilder(String fieldName, Geometry shape);
/** creates a new ShapeQueryBuilder from the provided field name, supplier, indexed shape id, and indexed shape type */
/** creates a new ShapeQueryBuilder from the provided field name, supplier and indexed shape id*/
protected abstract AbstractGeometryQueryBuilder<QB> newShapeQueryBuilder(String fieldName, Supplier<Geometry> shapeSupplier,
String indexedShapeId, String indexedShapeType);
String indexedShapeId);

/** returns true if the provided field type is valid for this query */
protected boolean isValidContentType(String typeName) {
Expand Down Expand Up @@ -469,9 +443,6 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep
} else {
builder.startObject(INDEXED_SHAPE_FIELD.getPreferredName())
.field(SHAPE_ID_FIELD.getPreferredName(), indexedShapeId);
if (indexedShapeType != null) {
builder.field(SHAPE_TYPE_FIELD.getPreferredName(), indexedShapeType);
}
if (indexedShapeIndex != null) {
builder.field(SHAPE_INDEX_FIELD.getPreferredName(), indexedShapeIndex);
}
Expand Down Expand Up @@ -503,7 +474,6 @@ protected boolean doEquals(AbstractGeometryQueryBuilder other) {
&& Objects.equals(indexedShapeId, other.indexedShapeId)
&& Objects.equals(indexedShapeIndex, other.indexedShapeIndex)
&& Objects.equals(indexedShapePath, other.indexedShapePath)
&& Objects.equals(indexedShapeType, other.indexedShapeType)
&& Objects.equals(indexedShapeRouting, other.indexedShapeRouting)
&& Objects.equals(relation, other.relation)
&& Objects.equals(shape, other.shape)
Expand All @@ -514,7 +484,7 @@ protected boolean doEquals(AbstractGeometryQueryBuilder other) {
@Override
protected int doHashCode() {
return Objects.hash(fieldName, indexedShapeId, indexedShapeIndex,
indexedShapePath, indexedShapeType, indexedShapeRouting, relation, shape, ignoreUnmapped, supplier);
indexedShapePath, indexedShapeRouting, relation, shape, ignoreUnmapped, supplier);
}

@Override
Expand All @@ -524,19 +494,14 @@ protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) throws
} else if (this.shape == null) {
SetOnce<Geometry> supplier = new SetOnce<>();
queryRewriteContext.registerAsyncAction((client, listener) -> {
GetRequest getRequest;
if (indexedShapeType == null) {
getRequest = new GetRequest(indexedShapeIndex, indexedShapeId);
} else {
getRequest = new GetRequest(indexedShapeIndex, indexedShapeId);
}
GetRequest getRequest = new GetRequest(indexedShapeIndex, indexedShapeId);
getRequest.routing(indexedShapeRouting);
fetch(client, getRequest, indexedShapePath, ActionListener.wrap(builder-> {
supplier.set(builder);
listener.onResponse(null);
}, listener::onFailure));
});
return newShapeQueryBuilder(this.fieldName, supplier::get, this.indexedShapeId, this.indexedShapeType).relation(relation);
return newShapeQueryBuilder(this.fieldName, supplier::get, this.indexedShapeId).relation(relation);
}
return this;
}
Expand All @@ -548,7 +513,6 @@ protected abstract static class ParsedGeometryQueryParams {
public ShapeBuilder shape;

public String id = null;
public String type = null;
public String index = null;
public String shapePath = null;
public String shapeRouting = null;
Expand Down Expand Up @@ -592,8 +556,6 @@ public static ParsedGeometryQueryParams parsedParamsFromXContent(XContentParser
} else if (token.isValue()) {
if (SHAPE_ID_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
params.id = parser.text();
} else if (SHAPE_TYPE_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
params.type = parser.text();
} else if (SHAPE_INDEX_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
params.index = parser.text();
} else if (SHAPE_PATH_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
Expand Down
34 changes: 9 additions & 25 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 @@ -22,7 +22,6 @@
import org.apache.logging.log4j.LogManager;
import org.apache.lucene.search.ConstantScoreQuery;
import org.apache.lucene.search.Query;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.common.geo.ShapeRelation;
Expand Down Expand Up @@ -86,27 +85,15 @@ 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);
}

/**
* Creates a new GeoShapeQueryBuilder whose Query will be against the given
* field name and will use the Shape found with the given ID in the given
* type
*
* @param fieldName
* Name of the field that will be filtered
* @param indexedShapeId
* ID of the indexed Shape that will be used in the Query
* @param indexedShapeType
* Index type of the indexed Shapes
* @deprecated use {@link #GeoShapeQueryBuilder(String, String)} instead
* field name and will use the Shape found with the given shape id and supplier
* @param fieldName Name of the field that will be queried
* @param shapeSupplier A shape supplier
* @param indexedShapeId The indexed id of a shape
*/
@Deprecated
public GeoShapeQueryBuilder(String fieldName, String indexedShapeId, String indexedShapeType) {
super(fieldName, indexedShapeId, indexedShapeType);
public GeoShapeQueryBuilder(String fieldName, Supplier<Geometry> shapeSupplier, String indexedShapeId) {
super(fieldName, shapeSupplier, indexedShapeId);
}

/**
Expand Down Expand Up @@ -204,8 +191,8 @@ protected GeoShapeQueryBuilder newShapeQueryBuilder(String fieldName, Geometry s

@Override
protected GeoShapeQueryBuilder newShapeQueryBuilder(String fieldName, Supplier<Geometry> shapeSupplier,
String indexedShapeId, String indexedShapeType) {
return new GeoShapeQueryBuilder(fieldName, shapeSupplier, indexedShapeId, indexedShapeType);
String indexedShapeId) {
return new GeoShapeQueryBuilder(fieldName, shapeSupplier, indexedShapeId);
}

@Override
Expand Down Expand Up @@ -265,14 +252,11 @@ public static GeoShapeQueryBuilder fromXContent(XContentParser parser) throws IO
(ParsedGeoShapeQueryParams) AbstractGeometryQueryBuilder.parsedParamsFromXContent(parser, new ParsedGeoShapeQueryParams());

GeoShapeQueryBuilder builder;
if (pgsqp.type != null) {
deprecationLogger.deprecatedAndMaybeLog("geo_share_query_with_types", TYPES_DEPRECATION_MESSAGE);
}

if (pgsqp.shape != null) {
builder = new GeoShapeQueryBuilder(pgsqp.fieldName, pgsqp.shape);
} else {
builder = new GeoShapeQueryBuilder(pgsqp.fieldName, pgsqp.id, pgsqp.type);
builder = new GeoShapeQueryBuilder(pgsqp.fieldName, pgsqp.id);
}

if (pgsqp.index != null) {
Expand Down
12 changes: 2 additions & 10 deletions server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@
public class GeoShapeQueryBuilderTests extends AbstractQueryTestCase<GeoShapeQueryBuilder> {

protected static String indexedShapeId;
protected static String indexedShapeType;
protected static String indexedShapePath;
protected static String indexedShapeIndex;
protected static String indexedShapeRouting;
Expand Down Expand Up @@ -94,8 +93,7 @@ protected GeoShapeQueryBuilder doCreateTestQueryBuilder(boolean indexedShape) {
} else {
indexedShapeToReturn = shape;
indexedShapeId = randomAlphaOfLengthBetween(3, 20);
indexedShapeType = randomBoolean() ? randomAlphaOfLengthBetween(3, 20) : null;
builder = new GeoShapeQueryBuilder(fieldName(), indexedShapeId, indexedShapeType);
builder = new GeoShapeQueryBuilder(fieldName(), indexedShapeId);
if (randomBoolean()) {
indexedShapeIndex = randomAlphaOfLengthBetween(3, 20);
builder.indexedShapeIndex(indexedShapeIndex);
Expand Down Expand Up @@ -154,7 +152,6 @@ protected GetResponse executeGet(GetRequest getRequest) {
public void clearShapeFields() {
indexedShapeToReturn = null;
indexedShapeId = null;
indexedShapeType = null;
indexedShapePath = null;
indexedShapeIndex = null;
indexedShapeRouting = null;
Expand All @@ -181,7 +178,7 @@ public void testNoShape() throws IOException {

public void testNoIndexedShape() throws IOException {
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
() -> new GeoShapeQueryBuilder(fieldName(), null, "type"));
() -> new GeoShapeQueryBuilder(fieldName(), null, null));
assertEquals("either shape or indexedShapeId is required", e.getMessage());
}

Expand Down Expand Up @@ -297,11 +294,6 @@ public void testSerializationFailsUnlessFetched() throws IOException {
protected QueryBuilder parseQuery(XContentParser parser) throws IOException {
QueryBuilder query = super.parseQuery(parser);
assertThat(query, instanceOf(GeoShapeQueryBuilder.class));

GeoShapeQueryBuilder shapeQuery = (GeoShapeQueryBuilder) query;
if (shapeQuery.indexedShapeType() != null) {
assertWarnings(GeoShapeQueryBuilder.TYPES_DEPRECATION_MESSAGE);
}
return query;
}
}
Loading

0 comments on commit 0ed05a9

Please sign in to comment.