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

[Remove] Type mappings from GeoShapeQueryBuilder (#2322) 

* Remove type end-points from GeoShapeBuilder

Signed-off-by: Suraj Singh <surajrider@gmail.com>

* Fix integration test failures

Signed-off-by: Suraj Singh <surajrider@gmail.com>
This commit is contained in:
Suraj Singh 2022-03-04 16:40:47 -08:00 committed by GitHub
parent be64af2f90
commit d5e58a2f3d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 32 additions and 153 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
server/src/internalClusterTest/java/org/opensearch/search/geo/GeoShapeIntegrationIT.java
View File

@ -240,7 +240,7 @@ public class GeoShapeIntegrationIT extends OpenSearchIntegTestCase {
indexRandom(true, client().prepareIndex("test").setId("0").setSource(source, XContentType.JSON).setRouting("ABC"));
SearchResponse searchResponse = client().prepareSearch("test")
.setQuery(geoShapeQuery("shape", "0", "doc").indexedShapeIndex("test").indexedShapeRouting("ABC"))
.setQuery(geoShapeQuery("shape", "0").indexedShapeIndex("test").indexedShapeRouting("ABC"))
.get();
assertThat(searchResponse.getHits().getTotalHits().value, equalTo(1L));

2
server/src/internalClusterTest/java/org/opensearch/search/geo/LegacyGeoShapeIntegrationIT.java
View File

@ -218,7 +218,7 @@ public class LegacyGeoShapeIntegrationIT extends OpenSearchIntegTestCase {
indexRandom(true, client().prepareIndex("test").setId("0").setSource(source, XContentType.JSON).setRouting("ABC"));
SearchResponse searchResponse = client().prepareSearch("test")
.setQuery(geoShapeQuery("shape", "0", "doc").indexedShapeIndex("test").indexedShapeRouting("ABC"))
.setQuery(geoShapeQuery("shape", "0").indexedShapeIndex("test").indexedShapeRouting("ABC"))
.get();
assertThat(searchResponse.getHits().getTotalHits().value, equalTo(1L));

86
server/src/main/java/org/opensearch/index/query/AbstractGeometryQueryBuilder.java
View File

@ -35,11 +35,11 @@ package org.opensearch.index.query;
import org.apache.lucene.search.MatchNoDocsQuery;
import org.apache.lucene.search.Query;
import org.apache.lucene.util.SetOnce;
import org.opensearch.Version;
import org.opensearch.action.ActionListener;
import org.opensearch.action.get.GetRequest;
import org.opensearch.action.get.GetResponse;
import org.opensearch.client.Client;
import org.opensearch.common.Nullable;
import org.opensearch.common.ParseField;
import org.opensearch.common.ParsingException;
import org.opensearch.common.geo.GeoJson;
@ -56,6 +56,7 @@ import org.opensearch.common.xcontent.XContentHelper;
import org.opensearch.common.xcontent.XContentParser;
import org.opensearch.geometry.Geometry;
import org.opensearch.index.mapper.MappedFieldType;
import org.opensearch.index.mapper.MapperService;
import java.io.IOException;
import java.util.Objects;
@ -66,9 +67,6 @@ import java.util.function.Supplier;
*/
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;
@ -80,7 +78,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");
@ -90,7 +87,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;
@ -113,7 +109,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);
}
/**
@ -126,7 +122,7 @@ public abstract class AbstractGeometryQueryBuilder<QB extends AbstractGeometryQu
* Shape used in the Query
*/
public AbstractGeometryQueryBuilder(String fieldName, Geometry shape) {
this(fieldName, shape, null, null);
this(fieldName, shape, null);
}
/**
@ -139,28 +135,10 @@ public abstract class AbstractGeometryQueryBuilder<QB extends AbstractGeometryQu
* 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");
}
@ -170,21 +148,21 @@ public abstract class AbstractGeometryQueryBuilder<QB extends AbstractGeometryQu
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;
}
/**
@ -196,11 +174,13 @@ public abstract class AbstractGeometryQueryBuilder<QB extends AbstractGeometryQu
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_2_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();
@ -222,7 +202,9 @@ public abstract class AbstractGeometryQueryBuilder<QB extends AbstractGeometryQu
GeometryIO.writeGeometry(out, shape);
} else {
out.writeOptionalString(indexedShapeId);
out.writeOptionalString(indexedShapeType);
if (out.getVersion().before(Version.V_2_0_0)) {
out.writeOptionalString(MapperService.SINGLE_MAPPING_NAME);
}
out.writeOptionalString(indexedShapeIndex);
out.writeOptionalString(indexedShapePath);
out.writeOptionalString(indexedShapeRouting);
@ -266,17 +248,6 @@ public abstract class AbstractGeometryQueryBuilder<QB extends AbstractGeometryQu
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
*
@ -382,12 +353,11 @@ public abstract class AbstractGeometryQueryBuilder<QB extends AbstractGeometryQu
/** 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, indexed shape id */
protected abstract AbstractGeometryQueryBuilder<QB> newShapeQueryBuilder(
String fieldName,
Supplier<Geometry> shapeSupplier,
String indexedShapeId,
String indexedShapeType
String indexedShapeId
);
@Override
@ -480,9 +450,6 @@ public abstract class AbstractGeometryQueryBuilder<QB extends AbstractGeometryQu
GeoJson.toXContent(shape, builder, params);
} 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);
}
@ -514,7 +481,6 @@ public abstract class AbstractGeometryQueryBuilder<QB extends AbstractGeometryQu
&& 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)
@ -529,7 +495,6 @@ public abstract class AbstractGeometryQueryBuilder<QB extends AbstractGeometryQu
indexedShapeId,
indexedShapeIndex,
indexedShapePath,
indexedShapeType,
indexedShapeRouting,
relation,
shape,
@ -552,7 +517,7 @@ public abstract class AbstractGeometryQueryBuilder<QB extends AbstractGeometryQu
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;
}
@ -564,7 +529,6 @@ public abstract class AbstractGeometryQueryBuilder<QB extends AbstractGeometryQu
public ShapeBuilder shape;
public String id = null;
public String type = null;
public String index = null;
public String shapePath = null;
public String shapeRouting = null;
@ -608,8 +572,6 @@ public abstract class AbstractGeometryQueryBuilder<QB extends AbstractGeometryQu
} 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())) {

45
server/src/main/java/org/opensearch/index/query/GeoShapeQueryBuilder.java
View File

@ -34,7 +34,6 @@ package org.opensearch.index.query;
import org.apache.lucene.search.ConstantScoreQuery;
import org.apache.lucene.search.Query;
import org.opensearch.common.Nullable;
import org.opensearch.common.ParseField;
import org.opensearch.common.ParsingException;
import org.opensearch.common.geo.ShapeRelation;
@ -43,7 +42,6 @@ import org.opensearch.common.geo.builders.ShapeBuilder;
import org.opensearch.common.geo.parsers.ShapeParser;
import org.opensearch.common.io.stream.StreamInput;
import org.opensearch.common.io.stream.StreamOutput;
import org.opensearch.common.logging.DeprecationLogger;
import org.opensearch.common.xcontent.XContentBuilder;
import org.opensearch.common.xcontent.XContentParser;
import org.opensearch.geometry.Geometry;
@ -62,8 +60,6 @@ import java.util.function.Supplier;
*/
public class GeoShapeQueryBuilder extends AbstractGeometryQueryBuilder<GeoShapeQueryBuilder> {
public static final String NAME = "geo_shape";
private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(GeoShapeQueryBuilder.class);
protected static final ParseField STRATEGY_FIELD = new ParseField("strategy");
private SpatialStrategy strategy;
@ -97,31 +93,8 @@ public class GeoShapeQueryBuilder extends AbstractGeometryQueryBuilder<GeoShapeQ
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
*/
@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);
}
/**
@ -223,13 +196,8 @@ public class GeoShapeQueryBuilder extends AbstractGeometryQueryBuilder<GeoShapeQ
}
@Override
protected GeoShapeQueryBuilder newShapeQueryBuilder(
String fieldName,
Supplier<Geometry> shapeSupplier,
String indexedShapeId,
String indexedShapeType
) {
return new GeoShapeQueryBuilder(fieldName, shapeSupplier, indexedShapeId, indexedShapeType);
protected GeoShapeQueryBuilder newShapeQueryBuilder(String fieldName, Supplier<Geometry> shapeSupplier, String indexedShapeId) {
return new GeoShapeQueryBuilder(fieldName, shapeSupplier, indexedShapeId);
}
@Override
@ -291,14 +259,11 @@ public class GeoShapeQueryBuilder extends AbstractGeometryQueryBuilder<GeoShapeQ
);
GeoShapeQueryBuilder builder;
if (pgsqp.type != null) {
deprecationLogger.deprecate("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) {

38
server/src/main/java/org/opensearch/index/query/QueryBuilders.java
View File

@ -686,14 +686,6 @@ public final class QueryBuilders {
return new GeoShapeQueryBuilder(name, indexedShapeId);
}
/**
* @deprecated Types are in the process of being removed, use {@link #geoShapeQuery(String, String)} instead.
*/
@Deprecated
public static GeoShapeQueryBuilder geoShapeQuery(String name, String indexedShapeId, String indexedShapeType) {
return new GeoShapeQueryBuilder(name, indexedShapeId, indexedShapeType);
}
/**
* A filter to filter indexed shapes intersecting with shapes
*
@ -722,16 +714,6 @@ public final class QueryBuilders {
return builder;
}
/**
* @deprecated Types are in the process of being removed, use {@link #geoIntersectionQuery(String, String)} instead.
*/
@Deprecated
public static GeoShapeQueryBuilder geoIntersectionQuery(String name, String indexedShapeId, String indexedShapeType) {
GeoShapeQueryBuilder builder = geoShapeQuery(name, indexedShapeId, indexedShapeType);
builder.relation(ShapeRelation.INTERSECTS);
return builder;
}
/**
* A filter to filter indexed shapes that are contained by a shape
*
@ -760,16 +742,6 @@ public final class QueryBuilders {
return builder;
}
/**
* @deprecated Types are in the process of being removed, use {@link #geoWithinQuery(String, String)} instead.
*/
@Deprecated
public static GeoShapeQueryBuilder geoWithinQuery(String name, String indexedShapeId, String indexedShapeType) {
GeoShapeQueryBuilder builder = geoShapeQuery(name, indexedShapeId, indexedShapeType);
builder.relation(ShapeRelation.WITHIN);
return builder;
}
/**
* A filter to filter indexed shapes that are not intersection with the query shape
*
@ -798,16 +770,6 @@ public final class QueryBuilders {
return builder;
}
/**
* @deprecated Types are in the process of being removed, use {@link #geoDisjointQuery(String, String)} instead.
*/
@Deprecated
public static GeoShapeQueryBuilder geoDisjointQuery(String name, String indexedShapeId, String indexedShapeType) {
GeoShapeQueryBuilder builder = geoShapeQuery(name, indexedShapeId, indexedShapeType);
builder.relation(ShapeRelation.DISJOINT);
return builder;
}
/**
* A filter to filter only documents where a field exists in them.
*

12
server/src/test/java/org/opensearch/index/query/GeoShapeQueryBuilderTests.java
View File

@ -69,7 +69,6 @@ import static org.hamcrest.Matchers.equalTo;
public abstract 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;
@ -119,7 +118,6 @@ public abstract class GeoShapeQueryBuilderTests extends AbstractQueryTestCase<Ge
public void clearShapeFields() {
indexedShapeToReturn = null;
indexedShapeId = null;
indexedShapeType = null;
indexedShapePath = null;
indexedShapeIndex = null;
indexedShapeRouting = null;
@ -145,10 +143,7 @@ public abstract class GeoShapeQueryBuilderTests extends AbstractQueryTestCase<Ge
}
public void testNoIndexedShape() throws IOException {
IllegalArgumentException e = expectThrows(
IllegalArgumentException.class,
() -> new GeoShapeQueryBuilder(fieldName(), null, "type")
);
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> new GeoShapeQueryBuilder(fieldName(), null, null));
assertEquals("either shape or indexedShapeId is required", e.getMessage());
}
@ -259,11 +254,6 @@ public abstract class GeoShapeQueryBuilderTests extends AbstractQueryTestCase<Ge
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;
}
}