Skip to content
This repository was archived by the owner on Apr 11, 2024. It is now read-only.

Commit d5e58a2

Browse files
dreamer-89dreamer-89
authored
[Remove] Type mappings from GeoShapeQueryBuilder (opensearch-project#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>
1 parent be64af2 commit d5e58a2

6 files changed

Lines changed: 32 additions & 153 deletions

File tree

‎server/src/internalClusterTest/java/org/opensearch/search/geo/GeoShapeIntegrationIT.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ public void testIndexShapeRouting() throws Exception {
240240
indexRandom(true, client().prepareIndex("test").setId("0").setSource(source, XContentType.JSON).setRouting("ABC"));
241241

242242
SearchResponse searchResponse = client().prepareSearch("test")
243-
.setQuery(geoShapeQuery("shape", "0", "doc").indexedShapeIndex("test").indexedShapeRouting("ABC"))
243+
.setQuery(geoShapeQuery("shape", "0").indexedShapeIndex("test").indexedShapeRouting("ABC"))
244244
.get();
245245

246246
assertThat(searchResponse.getHits().getTotalHits().value, equalTo(1L));

‎server/src/internalClusterTest/java/org/opensearch/search/geo/LegacyGeoShapeIntegrationIT.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ public void testIndexShapeRouting() throws Exception {
218218
indexRandom(true, client().prepareIndex("test").setId("0").setSource(source, XContentType.JSON).setRouting("ABC"));
219219

220220
SearchResponse searchResponse = client().prepareSearch("test")
221-
.setQuery(geoShapeQuery("shape", "0", "doc").indexedShapeIndex("test").indexedShapeRouting("ABC"))
221+
.setQuery(geoShapeQuery("shape", "0").indexedShapeIndex("test").indexedShapeRouting("ABC"))
222222
.get();
223223

224224
assertThat(searchResponse.getHits().getTotalHits().value, equalTo(1L));

‎server/src/main/java/org/opensearch/index/query/AbstractGeometryQueryBuilder.java‎

Lines changed: 24 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,11 @@
3535
import org.apache.lucene.search.MatchNoDocsQuery;
3636
import org.apache.lucene.search.Query;
3737
import org.apache.lucene.util.SetOnce;
38+
import org.opensearch.Version;
3839
import org.opensearch.action.ActionListener;
3940
import org.opensearch.action.get.GetRequest;
4041
import org.opensearch.action.get.GetResponse;
4142
import org.opensearch.client.Client;
42-
import org.opensearch.common.Nullable;
4343
import org.opensearch.common.ParseField;
4444
import org.opensearch.common.ParsingException;
4545
import org.opensearch.common.geo.GeoJson;
@@ -56,6 +56,7 @@
5656
import org.opensearch.common.xcontent.XContentParser;
5757
import org.opensearch.geometry.Geometry;
5858
import org.opensearch.index.mapper.MappedFieldType;
59+
import org.opensearch.index.mapper.MapperService;
5960

6061
import java.io.IOException;
6162
import java.util.Objects;
@@ -66,9 +67,6 @@
6667
*/
6768
public abstract class AbstractGeometryQueryBuilder<QB extends AbstractGeometryQueryBuilder<QB>> extends AbstractQueryBuilder<QB> {
6869

69-
static final String TYPES_DEPRECATION_MESSAGE = "[types removal] Types are deprecated in [geo_shape] queries. "
70-
+ "The type should no longer be specified in the [indexed_shape] section.";
71-
7270
public static final String DEFAULT_SHAPE_INDEX_NAME = "shapes";
7371
public static final String DEFAULT_SHAPE_FIELD_NAME = "shape";
7472
public static final ShapeRelation DEFAULT_SHAPE_RELATION = ShapeRelation.INTERSECTS;
@@ -80,7 +78,6 @@ public abstract class AbstractGeometryQueryBuilder<QB extends AbstractGeometryQu
8078
protected static final ParseField RELATION_FIELD = new ParseField("relation");
8179
protected static final ParseField INDEXED_SHAPE_FIELD = new ParseField("indexed_shape");
8280
protected static final ParseField SHAPE_ID_FIELD = new ParseField("id");
83-
protected static final ParseField SHAPE_TYPE_FIELD = new ParseField("type");
8481
protected static final ParseField SHAPE_INDEX_FIELD = new ParseField("index");
8582
protected static final ParseField SHAPE_PATH_FIELD = new ParseField("path");
8683
protected static final ParseField SHAPE_ROUTING_FIELD = new ParseField("routing");
@@ -90,7 +87,6 @@ public abstract class AbstractGeometryQueryBuilder<QB extends AbstractGeometryQu
9087
protected final Supplier<Geometry> supplier;
9188

9289
protected final String indexedShapeId;
93-
protected final String indexedShapeType;
9490

9591
protected Geometry shape;
9692
protected String indexedShapeIndex = DEFAULT_SHAPE_INDEX_NAME;
@@ -113,7 +109,7 @@ public abstract class AbstractGeometryQueryBuilder<QB extends AbstractGeometryQu
113109
*/
114110
@Deprecated
115111
protected AbstractGeometryQueryBuilder(String fieldName, ShapeBuilder shape) {
116-
this(fieldName, shape == null ? null : shape.buildGeometry(), null, null);
112+
this(fieldName, shape == null ? null : shape.buildGeometry(), null);
117113
}
118114

119115
/**
@@ -126,7 +122,7 @@ protected AbstractGeometryQueryBuilder(String fieldName, ShapeBuilder shape) {
126122
* Shape used in the Query
127123
*/
128124
public AbstractGeometryQueryBuilder(String fieldName, Geometry shape) {
129-
this(fieldName, shape, null, null);
125+
this(fieldName, shape, null);
130126
}
131127

132128
/**
@@ -139,28 +135,10 @@ public AbstractGeometryQueryBuilder(String fieldName, Geometry shape) {
139135
* ID of the indexed Shape that will be used in the Query
140136
*/
141137
protected AbstractGeometryQueryBuilder(String fieldName, String indexedShapeId) {
142-
this(fieldName, (Geometry) null, indexedShapeId, null);
138+
this(fieldName, (Geometry) null, indexedShapeId);
143139
}
144140

145-
/**
146-
* Creates a new AbstractGeometryQueryBuilder whose Query will be against the given
147-
* field name and will use the Shape found with the given ID in the given
148-
* type
149-
*
150-
* @param fieldName
151-
* Name of the field that will be filtered
152-
* @param indexedShapeId
153-
* ID of the indexed Shape that will be used in the Query
154-
* @param indexedShapeType
155-
* Index type of the indexed Shapes
156-
* @deprecated use {@link #AbstractGeometryQueryBuilder(String, String)} instead
157-
*/
158-
@Deprecated
159-
protected AbstractGeometryQueryBuilder(String fieldName, String indexedShapeId, String indexedShapeType) {
160-
this(fieldName, (Geometry) null, indexedShapeId, indexedShapeType);
161-
}
162-
163-
protected AbstractGeometryQueryBuilder(String fieldName, Geometry shape, String indexedShapeId, @Nullable String indexedShapeType) {
141+
protected AbstractGeometryQueryBuilder(String fieldName, Geometry shape, String indexedShapeId) {
164142
if (fieldName == null) {
165143
throw new IllegalArgumentException("fieldName is required");
166144
}
@@ -170,21 +148,21 @@ protected AbstractGeometryQueryBuilder(String fieldName, Geometry shape, String
170148
this.fieldName = fieldName;
171149
this.shape = shape;
172150
this.indexedShapeId = indexedShapeId;
173-
this.indexedShapeType = indexedShapeType;
174151
this.supplier = null;
175152
}
176153

177-
protected AbstractGeometryQueryBuilder(
178-
String fieldName,
179-
Supplier<Geometry> supplier,
180-
String indexedShapeId,
181-
@Nullable String indexedShapeType
182-
) {
154+
protected AbstractGeometryQueryBuilder(String fieldName, Supplier<Geometry> supplier, String indexedShapeId) {
155+
if (fieldName == null) {
156+
throw new IllegalArgumentException("fieldName is required");
157+
}
158+
if (supplier == null && indexedShapeId == null) {
159+
throw new IllegalArgumentException("either shape or indexedShapeId is required");
160+
}
161+
183162
this.fieldName = fieldName;
184163
this.shape = null;
185164
this.supplier = supplier;
186165
this.indexedShapeId = indexedShapeId;
187-
this.indexedShapeType = indexedShapeType;
188166
}
189167

190168
/**
@@ -196,11 +174,13 @@ protected AbstractGeometryQueryBuilder(StreamInput in) throws IOException {
196174
if (in.readBoolean()) {
197175
shape = GeometryIO.readGeometry(in);
198176
indexedShapeId = null;
199-
indexedShapeType = null;
200177
} else {
201178
shape = null;
202179
indexedShapeId = in.readOptionalString();
203-
indexedShapeType = in.readOptionalString();
180+
if (in.getVersion().before(Version.V_2_0_0)) {
181+
String type = in.readOptionalString();
182+
assert MapperService.SINGLE_MAPPING_NAME.equals(type) : "Expected type [_doc], got [" + type + "]";
183+
}
204184
indexedShapeIndex = in.readOptionalString();
205185
indexedShapePath = in.readOptionalString();
206186
indexedShapeRouting = in.readOptionalString();
@@ -222,7 +202,9 @@ protected void doWriteTo(StreamOutput out) throws IOException {
222202
GeometryIO.writeGeometry(out, shape);
223203
} else {
224204
out.writeOptionalString(indexedShapeId);
225-
out.writeOptionalString(indexedShapeType);
205+
if (out.getVersion().before(Version.V_2_0_0)) {
206+
out.writeOptionalString(MapperService.SINGLE_MAPPING_NAME);
207+
}
226208
out.writeOptionalString(indexedShapeIndex);
227209
out.writeOptionalString(indexedShapePath);
228210
out.writeOptionalString(indexedShapeRouting);
@@ -266,17 +248,6 @@ public String indexedShapeId() {
266248
return indexedShapeId;
267249
}
268250

269-
/**
270-
* @return the document type of the indexed Shape that will be used in the
271-
* Query
272-
*
273-
* @deprecated Types are in the process of being removed.
274-
*/
275-
@Deprecated
276-
public String indexedShapeType() {
277-
return indexedShapeType;
278-
}
279-
280251
/**
281252
* Sets the name of the index where the indexed Shape can be found
282253
*
@@ -382,12 +353,11 @@ public boolean ignoreUnmapped() {
382353
/** creates a new ShapeQueryBuilder from the provided field name and shape builder */
383354
protected abstract AbstractGeometryQueryBuilder<QB> newShapeQueryBuilder(String fieldName, Geometry shape);
384355

385-
/** creates a new ShapeQueryBuilder from the provided field name, supplier, indexed shape id, and indexed shape type */
356+
/** creates a new ShapeQueryBuilder from the provided field name, supplier, indexed shape id */
386357
protected abstract AbstractGeometryQueryBuilder<QB> newShapeQueryBuilder(
387358
String fieldName,
388359
Supplier<Geometry> shapeSupplier,
389-
String indexedShapeId,
390-
String indexedShapeType
360+
String indexedShapeId
391361
);
392362

393363
@Override
@@ -480,9 +450,6 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep
480450
GeoJson.toXContent(shape, builder, params);
481451
} else {
482452
builder.startObject(INDEXED_SHAPE_FIELD.getPreferredName()).field(SHAPE_ID_FIELD.getPreferredName(), indexedShapeId);
483-
if (indexedShapeType != null) {
484-
builder.field(SHAPE_TYPE_FIELD.getPreferredName(), indexedShapeType);
485-
}
486453
if (indexedShapeIndex != null) {
487454
builder.field(SHAPE_INDEX_FIELD.getPreferredName(), indexedShapeIndex);
488455
}
@@ -514,7 +481,6 @@ protected boolean doEquals(AbstractGeometryQueryBuilder other) {
514481
&& Objects.equals(indexedShapeId, other.indexedShapeId)
515482
&& Objects.equals(indexedShapeIndex, other.indexedShapeIndex)
516483
&& Objects.equals(indexedShapePath, other.indexedShapePath)
517-
&& Objects.equals(indexedShapeType, other.indexedShapeType)
518484
&& Objects.equals(indexedShapeRouting, other.indexedShapeRouting)
519485
&& Objects.equals(relation, other.relation)
520486
&& Objects.equals(shape, other.shape)
@@ -529,7 +495,6 @@ protected int doHashCode() {
529495
indexedShapeId,
530496
indexedShapeIndex,
531497
indexedShapePath,
532-
indexedShapeType,
533498
indexedShapeRouting,
534499
relation,
535500
shape,
@@ -552,7 +517,7 @@ protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) throws
552517
listener.onResponse(null);
553518
}, listener::onFailure));
554519
});
555-
return newShapeQueryBuilder(this.fieldName, supplier::get, this.indexedShapeId, this.indexedShapeType).relation(relation);
520+
return newShapeQueryBuilder(this.fieldName, supplier::get, this.indexedShapeId).relation(relation);
556521
}
557522
return this;
558523
}
@@ -564,7 +529,6 @@ protected abstract static class ParsedGeometryQueryParams {
564529
public ShapeBuilder shape;
565530

566531
public String id = null;
567-
public String type = null;
568532
public String index = null;
569533
public String shapePath = null;
570534
public String shapeRouting = null;
@@ -608,8 +572,6 @@ public static ParsedGeometryQueryParams parsedParamsFromXContent(XContentParser
608572
} else if (token.isValue()) {
609573
if (SHAPE_ID_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
610574
params.id = parser.text();
611-
} else if (SHAPE_TYPE_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
612-
params.type = parser.text();
613575
} else if (SHAPE_INDEX_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
614576
params.index = parser.text();
615577
} else if (SHAPE_PATH_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {

‎server/src/main/java/org/opensearch/index/query/GeoShapeQueryBuilder.java‎

Lines changed: 5 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
3434

3535
import org.apache.lucene.search.ConstantScoreQuery;
3636
import org.apache.lucene.search.Query;
37-
import org.opensearch.common.Nullable;
3837
import org.opensearch.common.ParseField;
3938
import org.opensearch.common.ParsingException;
4039
import org.opensearch.common.geo.ShapeRelation;
@@ -43,7 +42,6 @@
4342
import org.opensearch.common.geo.parsers.ShapeParser;
4443
import org.opensearch.common.io.stream.StreamInput;
4544
import org.opensearch.common.io.stream.StreamOutput;
46-
import org.opensearch.common.logging.DeprecationLogger;
4745
import org.opensearch.common.xcontent.XContentBuilder;
4846
import org.opensearch.common.xcontent.XContentParser;
4947
import org.opensearch.geometry.Geometry;
@@ -62,8 +60,6 @@
6260
*/
6361
public class GeoShapeQueryBuilder extends AbstractGeometryQueryBuilder<GeoShapeQueryBuilder> {
6462
public static final String NAME = "geo_shape";
65-
private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(GeoShapeQueryBuilder.class);
66-
6763
protected static final ParseField STRATEGY_FIELD = new ParseField("strategy");
6864

6965
private SpatialStrategy strategy;
@@ -97,31 +93,8 @@ public GeoShapeQueryBuilder(String fieldName, ShapeBuilder shape) {
9793
super(fieldName, shape);
9894
}
9995

100-
public GeoShapeQueryBuilder(
101-
String fieldName,
102-
Supplier<Geometry> shapeSupplier,
103-
String indexedShapeId,
104-
@Nullable String indexedShapeType
105-
) {
106-
super(fieldName, shapeSupplier, indexedShapeId, indexedShapeType);
107-
}
108-
109-
/**
110-
* Creates a new GeoShapeQueryBuilder whose Query will be against the given
111-
* field name and will use the Shape found with the given ID in the given
112-
* type
113-
*
114-
* @param fieldName
115-
* Name of the field that will be filtered
116-
* @param indexedShapeId
117-
* ID of the indexed Shape that will be used in the Query
118-
* @param indexedShapeType
119-
* Index type of the indexed Shapes
120-
* @deprecated use {@link #GeoShapeQueryBuilder(String, String)} instead
121-
*/
122-
@Deprecated
123-
public GeoShapeQueryBuilder(String fieldName, String indexedShapeId, String indexedShapeType) {
124-
super(fieldName, indexedShapeId, indexedShapeType);
96+
public GeoShapeQueryBuilder(String fieldName, Supplier<Geometry> shapeSupplier, String indexedShapeId) {
97+
super(fieldName, shapeSupplier, indexedShapeId);
12598
}
12699

127100
/**
@@ -223,13 +196,8 @@ protected GeoShapeQueryBuilder newShapeQueryBuilder(String fieldName, Geometry s
223196
}
224197

225198
@Override
226-
protected GeoShapeQueryBuilder newShapeQueryBuilder(
227-
String fieldName,
228-
Supplier<Geometry> shapeSupplier,
229-
String indexedShapeId,
230-
String indexedShapeType
231-
) {
232-
return new GeoShapeQueryBuilder(fieldName, shapeSupplier, indexedShapeId, indexedShapeType);
199+
protected GeoShapeQueryBuilder newShapeQueryBuilder(String fieldName, Supplier<Geometry> shapeSupplier, String indexedShapeId) {
200+
return new GeoShapeQueryBuilder(fieldName, shapeSupplier, indexedShapeId);
233201
}
234202

235203
@Override
@@ -291,14 +259,11 @@ public static GeoShapeQueryBuilder fromXContent(XContentParser parser) throws IO
291259
);
292260

293261
GeoShapeQueryBuilder builder;
294-
if (pgsqp.type != null) {
295-
deprecationLogger.deprecate("geo_share_query_with_types", TYPES_DEPRECATION_MESSAGE);
296-
}
297262

298263
if (pgsqp.shape != null) {
299264
builder = new GeoShapeQueryBuilder(pgsqp.fieldName, pgsqp.shape);
300265
} else {
301-
builder = new GeoShapeQueryBuilder(pgsqp.fieldName, pgsqp.id, pgsqp.type);
266+
builder = new GeoShapeQueryBuilder(pgsqp.fieldName, pgsqp.id);
302267
}
303268

304269
if (pgsqp.index != null) {

0 commit comments

Comments
 (0)