diff --git a/server/src/internalClusterTest/java/org/opensearch/search/geo/GeoShapeIntegrationIT.java b/server/src/internalClusterTest/java/org/opensearch/search/geo/GeoShapeIntegrationIT.java index 74474788f3f14..2db5973a2aa85 100644 --- a/server/src/internalClusterTest/java/org/opensearch/search/geo/GeoShapeIntegrationIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/search/geo/GeoShapeIntegrationIT.java @@ -240,7 +240,7 @@ public void testIndexShapeRouting() throws Exception { 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)); diff --git a/server/src/internalClusterTest/java/org/opensearch/search/geo/LegacyGeoShapeIntegrationIT.java b/server/src/internalClusterTest/java/org/opensearch/search/geo/LegacyGeoShapeIntegrationIT.java index c566689bd6fb8..479fd00e5e08b 100644 --- a/server/src/internalClusterTest/java/org/opensearch/search/geo/LegacyGeoShapeIntegrationIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/search/geo/LegacyGeoShapeIntegrationIT.java @@ -218,7 +218,7 @@ public void testIndexShapeRouting() throws Exception { 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)); diff --git a/server/src/main/java/org/opensearch/index/query/AbstractGeometryQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/AbstractGeometryQueryBuilder.java index b5b4abdbaf118..9281f1767d72d 100644 --- a/server/src/main/java/org/opensearch/index/query/AbstractGeometryQueryBuilder.java +++ b/server/src/main/java/org/opensearch/index/query/AbstractGeometryQueryBuilder.java @@ -35,11 +35,11 @@ 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.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 @@ */ public abstract class AbstractGeometryQueryBuilder> extends AbstractQueryBuilder { - 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 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 supplier, - String indexedShapeId, - @Nullable String indexedShapeType - ) { + protected AbstractGeometryQueryBuilder(String fieldName, Supplier 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 @@ 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_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 @@ protected void doWriteTo(StreamOutput out) throws IOException { 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 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 * @@ -382,12 +353,11 @@ public boolean ignoreUnmapped() { /** creates a new ShapeQueryBuilder from the provided field name and shape builder */ protected abstract AbstractGeometryQueryBuilder 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 newShapeQueryBuilder( String fieldName, Supplier shapeSupplier, - String indexedShapeId, - String indexedShapeType + String indexedShapeId ); @Override @@ -480,9 +450,6 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep 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 @@ 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) @@ -529,7 +495,6 @@ protected int doHashCode() { indexedShapeId, indexedShapeIndex, indexedShapePath, - indexedShapeType, indexedShapeRouting, relation, shape, @@ -552,7 +517,7 @@ protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) throws 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 @@ 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; @@ -608,8 +572,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())) { diff --git a/server/src/main/java/org/opensearch/index/query/GeoShapeQueryBuilder.java b/server/src/main/java/org/opensearch/index/query/GeoShapeQueryBuilder.java index 246a1e1dcf921..161c6e64c7bf3 100644 --- a/server/src/main/java/org/opensearch/index/query/GeoShapeQueryBuilder.java +++ b/server/src/main/java/org/opensearch/index/query/GeoShapeQueryBuilder.java @@ -34,7 +34,6 @@ 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.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 @@ */ public class GeoShapeQueryBuilder extends AbstractGeometryQueryBuilder { 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 GeoShapeQueryBuilder(String fieldName, ShapeBuilder shape) { super(fieldName, shape); } - public GeoShapeQueryBuilder( - String fieldName, - Supplier 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 shapeSupplier, String indexedShapeId) { + super(fieldName, shapeSupplier, indexedShapeId); } /** @@ -223,13 +196,8 @@ protected GeoShapeQueryBuilder newShapeQueryBuilder(String fieldName, Geometry s } @Override - protected GeoShapeQueryBuilder newShapeQueryBuilder( - String fieldName, - Supplier shapeSupplier, - String indexedShapeId, - String indexedShapeType - ) { - return new GeoShapeQueryBuilder(fieldName, shapeSupplier, indexedShapeId, indexedShapeType); + protected GeoShapeQueryBuilder newShapeQueryBuilder(String fieldName, Supplier shapeSupplier, String indexedShapeId) { + return new GeoShapeQueryBuilder(fieldName, shapeSupplier, indexedShapeId); } @Override @@ -291,14 +259,11 @@ public static GeoShapeQueryBuilder fromXContent(XContentParser parser) throws IO ); 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) { diff --git a/server/src/main/java/org/opensearch/index/query/QueryBuilders.java b/server/src/main/java/org/opensearch/index/query/QueryBuilders.java index a80e4c1c9f745..7ea12fdc6406b 100644 --- a/server/src/main/java/org/opensearch/index/query/QueryBuilders.java +++ b/server/src/main/java/org/opensearch/index/query/QueryBuilders.java @@ -686,14 +686,6 @@ public static GeoShapeQueryBuilder geoShapeQuery(String name, String indexedShap 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 static GeoShapeQueryBuilder geoIntersectionQuery(String name, String inde 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 static GeoShapeQueryBuilder geoWithinQuery(String name, String indexedSha 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 static GeoShapeQueryBuilder geoDisjointQuery(String name, String indexedS 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. * diff --git a/server/src/test/java/org/opensearch/index/query/GeoShapeQueryBuilderTests.java b/server/src/test/java/org/opensearch/index/query/GeoShapeQueryBuilderTests.java index 3eab92d7e2112..05fee1c043557 100644 --- a/server/src/test/java/org/opensearch/index/query/GeoShapeQueryBuilderTests.java +++ b/server/src/test/java/org/opensearch/index/query/GeoShapeQueryBuilderTests.java @@ -69,7 +69,6 @@ public abstract class GeoShapeQueryBuilderTests extends AbstractQueryTestCase { protected static String indexedShapeId; - protected static String indexedShapeType; protected static String indexedShapePath; protected static String indexedShapeIndex; protected static String indexedShapeRouting; @@ -119,7 +118,6 @@ protected GetResponse executeGet(GetRequest getRequest) { public void clearShapeFields() { indexedShapeToReturn = null; indexedShapeId = null; - indexedShapeType = null; indexedShapePath = null; indexedShapeIndex = null; indexedShapeRouting = null; @@ -145,10 +143,7 @@ public void testNoShape() throws IOException { } 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 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; } }