Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Remove] Type mappings from GeoShapeQueryBuilder #2322

Merged
merged 2 commits into from
Mar 5, 2022
Merged

[Remove] Type mappings from GeoShapeQueryBuilder #2322

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
769e64b
Remove type end-points from GeoShapeBuilder
dreamer-89 Mar 2, 2022
8899103
Fix integration test failures
dreamer-89 Mar 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion server/src/internalClusterTest/java/org/opensearch/search/geo/GeoShapeIntegrationIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
2 changes: 1 addition & 1 deletion ...r/src/internalClusterTest/java/org/opensearch/search/geo/LegacyGeoShapeIntegrationIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
86 changes: 24 additions & 62 deletions server/src/main/java/org/opensearch/index/query/AbstractGeometryQueryBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -66,9 +67,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 @@ -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");
Expand All @@ -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;
Expand All @@ -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);
}

/**
Expand All @@ -126,7 +122,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 @@ -139,28 +135,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 @@ -170,21 +148,21 @@ 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;
}

/**
Expand All @@ -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();
Expand All @@ -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);
Expand Down Expand Up @@ -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
*
Expand Down Expand Up @@ -382,12 +353,11 @@ public boolean ignoreUnmapped() {
/** 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
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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)
Expand All @@ -529,7 +495,6 @@ protected int doHashCode() {
indexedShapeId,
indexedShapeIndex,
indexedShapePath,
indexedShapeType,
indexedShapeRouting,
relation,
shape,
Expand All @@ -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;
}
Expand All @@ -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;
Expand Down Expand Up @@ -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())) {
Expand Down
45 changes: 5 additions & 40 deletions server/src/main/java/org/opensearch/index/query/GeoShapeQueryBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -62,8 +60,6 @@
*/
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;
Expand Down Expand Up @@ -97,31 +93,8 @@ 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
*/
@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 @@ -223,13 +196,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);
protected GeoShapeQueryBuilder newShapeQueryBuilder(String fieldName, Supplier<Geometry> shapeSupplier, String indexedShapeId) {
return new GeoShapeQueryBuilder(fieldName, shapeSupplier, indexedShapeId);
}

@Override
Expand Down Expand Up @@ -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) {
Expand Down
Loading