From 8e892f5fde305e5bbc967285cdbef28168453b1b Mon Sep 17 00:00:00 2001 From: Mayya Sharipova Date: Mon, 24 Dec 2018 13:34:35 -0500 Subject: [PATCH 01/14] Deprecate reference to _type in lookup queries Relates to #35190 --- .../java/org/elasticsearch/client/CrudIT.java | 8 +- .../QueryDSLDocumentationTests.java | 14 +- .../query-dsl/geo-shape-query.asciidoc | 7 +- docs/java-api/query-dsl/ids-query.asciidoc | 1 - .../PercolatorFieldMapperTests.java | 1 + .../index/query/GeoShapeQueryBuilder.java | 45 ++- .../index/query/IdsQueryBuilder.java | 12 +- .../index/query/MoreLikeThisQueryBuilder.java | 38 ++ .../index/query/QueryBuilders.java | 5 + .../index/query/QueryShardContext.java | 4 +- .../elasticsearch/indices/TermsLookup.java | 61 ++- .../query/GeoShapeQueryBuilderTests.java | 24 +- .../query/GeoShapeQueryBuilderTypedTests.java | 233 ++++++++++++ .../index/query/IdsQueryBuilderTests.java | 83 +--- .../query/IdsQueryBuilderTypedTests.java | 192 ++++++++++ .../query/LegacyGeoShapeFieldQueryTests.java | 3 +- .../query/MoreLikeThisQueryBuilderTests.java | 7 +- .../MoreLikeThisQueryBuilderTypedTests.java | 359 ++++++++++++++++++ .../index/query/TermsQueryBuilderTests.java | 8 +- .../query/TermsQueryBuilderTypedTests.java | 148 ++++++++ .../test/AbstractQueryTestCase.java | 4 +- 21 files changed, 1114 insertions(+), 143 deletions(-) create mode 100644 server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTypedTests.java create mode 100644 server/src/test/java/org/elasticsearch/index/query/IdsQueryBuilderTypedTests.java create mode 100644 server/src/test/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilderTypedTests.java create mode 100644 server/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTypedTests.java diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java index 59135204c5be1..bdfc3fe4383ee 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/CrudIT.java @@ -875,7 +875,7 @@ public void testUpdateByQuery() throws Exception { // test1: create one doc in dest UpdateByQueryRequest updateByQueryRequest = new UpdateByQueryRequest(); updateByQueryRequest.indices(sourceIndex); - updateByQueryRequest.setQuery(new IdsQueryBuilder().addIds("1").types("_doc")); + updateByQueryRequest.setQuery(new IdsQueryBuilder().addIds("1")); updateByQueryRequest.setRefresh(true); BulkByScrollResponse bulkResponse = execute(updateByQueryRequest, highLevelClient()::updateByQuery, highLevelClient()::updateByQueryAsync); @@ -917,7 +917,7 @@ public void testUpdateByQuery() throws Exception { // test update-by-query rethrottling UpdateByQueryRequest updateByQueryRequest = new UpdateByQueryRequest(); updateByQueryRequest.indices(sourceIndex); - updateByQueryRequest.setQuery(new IdsQueryBuilder().addIds("1").types("_doc")); + updateByQueryRequest.setQuery(new IdsQueryBuilder().addIds("1")); updateByQueryRequest.setRefresh(true); // this following settings are supposed to halt reindexing after first document @@ -987,7 +987,7 @@ public void testDeleteByQuery() throws Exception { // test1: delete one doc DeleteByQueryRequest deleteByQueryRequest = new DeleteByQueryRequest(); deleteByQueryRequest.indices(sourceIndex); - deleteByQueryRequest.setQuery(new IdsQueryBuilder().addIds("1").types("_doc")); + deleteByQueryRequest.setQuery(new IdsQueryBuilder().addIds("1")); deleteByQueryRequest.setRefresh(true); BulkByScrollResponse bulkResponse = execute(deleteByQueryRequest, highLevelClient()::deleteByQuery, highLevelClient()::deleteByQueryAsync); @@ -1009,7 +1009,7 @@ public void testDeleteByQuery() throws Exception { // test delete-by-query rethrottling DeleteByQueryRequest deleteByQueryRequest = new DeleteByQueryRequest(); deleteByQueryRequest.indices(sourceIndex); - deleteByQueryRequest.setQuery(new IdsQueryBuilder().addIds("2", "3").types("_doc")); + deleteByQueryRequest.setQuery(new IdsQueryBuilder().addIds("2", "3")); deleteByQueryRequest.setRefresh(true); // this following settings are supposed to halt reindexing after first document diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/QueryDSLDocumentationTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/QueryDSLDocumentationTests.java index 789d237c5a3bc..3037e4b517eba 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/QueryDSLDocumentationTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/QueryDSLDocumentationTests.java @@ -207,11 +207,10 @@ public void testGeoShape() throws IOException { // Using pre-indexed shapes GeoShapeQueryBuilder qb = geoShapeQuery( "pin.location", // <1> - "DEU", // <2> - "countries"); // <3> - qb.relation(ShapeRelation.WITHIN) // <4> - .indexedShapeIndex("shapes") // <5> - .indexedShapePath("location"); // <6> + "DEU"); // <2> + qb.relation(ShapeRelation.WITHIN) // <3> + .indexedShapeIndex("shapes") // <4> + .indexedShapePath("location"); // <5> // end::indexed_geo_shape } } @@ -236,10 +235,7 @@ public void testHasParent() { public void testIds() { // tag::ids - idsQuery("my_type", "type2") - .addIds("1", "4", "100"); - - idsQuery() // <1> + idsQuery() .addIds("1", "4", "100"); // end::ids } diff --git a/docs/java-api/query-dsl/geo-shape-query.asciidoc b/docs/java-api/query-dsl/geo-shape-query.asciidoc index 803f1849b5cdf..c2cd4c14e3adc 100644 --- a/docs/java-api/query-dsl/geo-shape-query.asciidoc +++ b/docs/java-api/query-dsl/geo-shape-query.asciidoc @@ -51,7 +51,6 @@ include-tagged::{query-dsl-test}[indexed_geo_shape] -------------------------------------------------- <1> field <2> The ID of the document that containing the pre-indexed shape. -<3> Index type where the pre-indexed shape is. -<4> relation -<5> Name of the index where the pre-indexed shape is. Defaults to 'shapes'. -<6> The field specified as path containing the pre-indexed shape. Defaults to 'shape'. +<3> relation +<4> Name of the index where the pre-indexed shape is. Defaults to 'shapes'. +<5> The field specified as path containing the pre-indexed shape. Defaults to 'shape'. diff --git a/docs/java-api/query-dsl/ids-query.asciidoc b/docs/java-api/query-dsl/ids-query.asciidoc index 9abc8ed9fed7c..ba12a5df38b0e 100644 --- a/docs/java-api/query-dsl/ids-query.asciidoc +++ b/docs/java-api/query-dsl/ids-query.asciidoc @@ -8,4 +8,3 @@ See {ref}/query-dsl-ids-query.html[Ids Query] -------------------------------------------------- include-tagged::{query-dsl-test}[ids] -------------------------------------------------- -<1> type is optional diff --git a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorFieldMapperTests.java b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorFieldMapperTests.java index 3030e48690fe9..001a3d959858f 100644 --- a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorFieldMapperTests.java +++ b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorFieldMapperTests.java @@ -553,6 +553,7 @@ public void testQueryWithRewrite() throws Exception { PlainActionFuture future = new PlainActionFuture<>(); Rewriteable.rewriteAndFetch(queryBuilder, shardContext, future); assertQueryBuilder(qbSource, future.get()); + assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); } diff --git a/server/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java index 6ee0f3f10ddcc..0de62f361c7d3 100644 --- a/server/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java @@ -23,6 +23,7 @@ import org.apache.lucene.geo.Line; import org.apache.lucene.geo.Polygon; import org.apache.lucene.geo.Rectangle; +import org.apache.logging.log4j.LogManager; import org.apache.lucene.search.BooleanClause; import org.apache.lucene.search.BooleanQuery; import org.apache.lucene.search.ConstantScoreQuery; @@ -38,6 +39,7 @@ 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.GeoPoint; @@ -48,6 +50,7 @@ import org.elasticsearch.common.geo.parsers.ShapeParser; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContentBuilder; @@ -66,6 +69,7 @@ */ public class GeoShapeQueryBuilder extends AbstractQueryBuilder { public static final String NAME = "geo_shape"; + private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(GeoShapeQueryBuilder.class)); public static final String DEFAULT_SHAPE_INDEX_NAME = "shapes"; public static final String DEFAULT_SHAPE_FIELD_NAME = "shape"; @@ -119,6 +123,19 @@ public GeoShapeQueryBuilder(String fieldName, ShapeBuilder shape) { this(fieldName, shape, null, null); } + /** + * Creates a new GeoShapeQueryBuilder whose Query will be against the given + * field name and will use the Shape found with the given ID + * + * @param fieldName + * Name of the field that will be filtered + * @param indexedShapeId + * ID of the indexed Shape that will be used in the Query + */ + public GeoShapeQueryBuilder(String fieldName, String indexedShapeId) { + this(fieldName, (ShapeBuilder) null, indexedShapeId, null); + } + /** * 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 @@ -130,20 +147,19 @@ public GeoShapeQueryBuilder(String fieldName, ShapeBuilder shape) { * 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) { this(fieldName, (ShapeBuilder) null, indexedShapeId, indexedShapeType); } - private GeoShapeQueryBuilder(String fieldName, ShapeBuilder shape, String indexedShapeId, String indexedShapeType) { + private GeoShapeQueryBuilder(String fieldName, ShapeBuilder shape, String indexedShapeId, @Nullable String indexedShapeType) { if (fieldName == null) { throw new IllegalArgumentException("fieldName is required"); } if (shape == null && indexedShapeId == null) { - throw new IllegalArgumentException("either shapeBytes or indexedShapeId and indexedShapeType are required"); - } - if (indexedShapeId != null && indexedShapeType == null) { - throw new IllegalArgumentException("indexedShapeType is required if indexedShapeId is specified"); + throw new IllegalArgumentException("either shapeBytes or indexedShapeId is required"); } this.fieldName = fieldName; this.shape = shape; @@ -152,7 +168,8 @@ private GeoShapeQueryBuilder(String fieldName, ShapeBuilder shape, String indexe this.supplier = null; } - private GeoShapeQueryBuilder(String fieldName, Supplier supplier, String indexedShapeId, String indexedShapeType) { + private GeoShapeQueryBuilder(String fieldName, Supplier supplier, String indexedShapeId, + @Nullable String indexedShapeType) { this.fieldName = fieldName; this.shape = null; this.supplier = supplier; @@ -239,6 +256,7 @@ public String indexedShapeId() { * @return the document type of the indexed Shape that will be used in the * Query */ + @Deprecated public String indexedShapeType() { return indexedShapeType; } @@ -566,8 +584,10 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep shape.toXContent(builder, params); } else { builder.startObject(INDEXED_SHAPE_FIELD.getPreferredName()) - .field(SHAPE_ID_FIELD.getPreferredName(), indexedShapeId) - .field(SHAPE_TYPE_FIELD.getPreferredName(), indexedShapeType); + .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); } @@ -644,6 +664,8 @@ public static GeoShapeQueryBuilder fromXContent(XContentParser parser) throws IO if (SHAPE_ID_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { id = parser.text(); } else if (SHAPE_TYPE_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { + deprecationLogger.deprecatedAndMaybeLog( + "geo_share_query_with_types", QueryShardContext.TYPES_DEPRECATION_MESSAGE); type = parser.text(); } else if (SHAPE_INDEX_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { index = parser.text(); @@ -739,7 +761,12 @@ protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) throws } else if (this.shape == null) { SetOnce supplier = new SetOnce<>(); queryRewriteContext.registerAsyncAction((client, listener) -> { - GetRequest getRequest = new GetRequest(indexedShapeIndex, indexedShapeType, indexedShapeId); + GetRequest getRequest; + if (indexedShapeType == null) { + getRequest = new GetRequest(indexedShapeIndex, indexedShapeId); + } else { + getRequest = new GetRequest(indexedShapeIndex, indexedShapeType, indexedShapeId); + } getRequest.routing(indexedShapeRouting); fetch(client, getRequest, indexedShapePath, ActionListener.wrap(builder-> { supplier.set(builder); diff --git a/server/src/main/java/org/elasticsearch/index/query/IdsQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/IdsQueryBuilder.java index 7cbd38f3398fd..e4e79ffa2b144 100644 --- a/server/src/main/java/org/elasticsearch/index/query/IdsQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/IdsQueryBuilder.java @@ -19,6 +19,7 @@ package org.elasticsearch.index.query; +import org.apache.logging.log4j.LogManager; import org.apache.lucene.search.MatchNoDocsQuery; import org.apache.lucene.search.Query; import org.elasticsearch.cluster.metadata.MetaData; @@ -27,6 +28,7 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.lucene.search.Queries; import org.elasticsearch.common.xcontent.ObjectParser; import org.elasticsearch.common.xcontent.XContentBuilder; @@ -51,6 +53,7 @@ */ public class IdsQueryBuilder extends AbstractQueryBuilder { public static final String NAME = "ids"; + private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(IdsQueryBuilder.class)); private static final ParseField TYPE_FIELD = new ParseField("type"); private static final ParseField VALUES_FIELD = new ParseField("values"); @@ -84,11 +87,17 @@ protected void doWriteTo(StreamOutput out) throws IOException { /** * Add types to query */ - // TODO: Remove + // TODO: Remove in 8.0 + @Deprecated public IdsQueryBuilder types(String... types) { if (types == null) { throw new IllegalArgumentException("[" + NAME + "] types cannot be null"); } + // Even if types are null, IdsQueryBuilder uses an empty array for decoding types. + // For this reason, issue deprecation warning if types contain something. + if (types.length > 0) { + deprecationLogger.deprecatedAndMaybeLog("ids_query_with_types", QueryShardContext.TYPES_DEPRECATION_MESSAGE); + } this.types = types; return this; } @@ -96,6 +105,7 @@ public IdsQueryBuilder types(String... types) { /** * Returns the types used in this query */ + @Deprecated public String[] types() { return this.types; } diff --git a/server/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java index 6f87ac0fda79b..3e07d0efd7dfc 100644 --- a/server/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java @@ -19,6 +19,7 @@ package org.elasticsearch.index.query; +import org.apache.logging.log4j.LogManager; import org.apache.lucene.analysis.Analyzer; import org.apache.lucene.index.Fields; import org.apache.lucene.search.BooleanClause; @@ -41,6 +42,7 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.lucene.search.MoreLikeThisQuery; import org.elasticsearch.common.lucene.search.XMoreLikeThis; import org.elasticsearch.common.lucene.uid.Versions; @@ -76,6 +78,8 @@ */ public class MoreLikeThisQueryBuilder extends AbstractQueryBuilder { public static final String NAME = "more_like_this"; + private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(MoreLikeThisQueryBuilder.class)); + public static final int DEFAULT_MAX_QUERY_TERMS = XMoreLikeThis.DEFAULT_MAX_QUERY_TERMS; public static final int DEFAULT_MIN_TERM_FREQ = XMoreLikeThis.DEFAULT_MIN_TERM_FREQ; @@ -178,6 +182,36 @@ public Item() { this.versionType = copy.versionType; } + + /** + * Constructor for a given item / document request + * + * @param index the index where the document is located + * @param id and its id + */ + public Item(@Nullable String index,String id) { + if (id == null) { + throw new IllegalArgumentException("Item requires id to be non-null"); + } + this.index = index; + this.id = id; + } + + /** + * Constructor for an artificial document request, that is not present in the index. + * + * @param index the index to be used for parsing the doc + * @param doc the document specification + */ + public Item(@Nullable String index, XContentBuilder doc) { + if (doc == null) { + throw new IllegalArgumentException("Item requires doc to be non-null"); + } + this.index = index; + this.doc = BytesReference.bytes(doc); + this.xContentType = doc.contentType(); + } + /** * Constructor for a given item / document request * @@ -185,6 +219,7 @@ public Item() { * @param type the type of the document * @param id and its id */ + @Deprecated public Item(@Nullable String index, @Nullable String type, String id) { if (id == null) { throw new IllegalArgumentException("Item requires id to be non-null"); @@ -201,6 +236,7 @@ public Item(@Nullable String index, @Nullable String type, String id) { * @param type the type to be used for parsing the doc * @param doc the document specification */ + @Deprecated public Item(@Nullable String index, @Nullable String type, XContentBuilder doc) { if (doc == null) { throw new IllegalArgumentException("Item requires doc to be non-null"); @@ -362,6 +398,8 @@ public static Item parse(XContentParser parser, Item item) throws IOException { if (INDEX.match(currentFieldName, parser.getDeprecationHandler())) { item.index = parser.text(); } else if (TYPE.match(currentFieldName, parser.getDeprecationHandler())) { + deprecationLogger.deprecatedAndMaybeLog( + "more_like_this_query_with_types", QueryShardContext.TYPES_DEPRECATION_MESSAGE); item.type = parser.text(); } else if (ID.match(currentFieldName, parser.getDeprecationHandler())) { item.id = parser.text(); diff --git a/server/src/main/java/org/elasticsearch/index/query/QueryBuilders.java b/server/src/main/java/org/elasticsearch/index/query/QueryBuilders.java index f5cf2d5da66be..28f9cce16eab0 100644 --- a/server/src/main/java/org/elasticsearch/index/query/QueryBuilders.java +++ b/server/src/main/java/org/elasticsearch/index/query/QueryBuilders.java @@ -122,6 +122,7 @@ public static IdsQueryBuilder idsQuery() { * * @param types The mapping/doc type */ + @Deprecated public static IdsQueryBuilder idsQuery(String... types) { return new IdsQueryBuilder().types(types); } @@ -650,6 +651,10 @@ public static GeoShapeQueryBuilder geoShapeQuery(String name, String indexedShap return new GeoShapeQueryBuilder(name, indexedShapeId, indexedShapeType); } + public static GeoShapeQueryBuilder geoShapeQuery(String name, String indexedShapeId) { + return new GeoShapeQueryBuilder(name, indexedShapeId); + } + /** * A filter to filter indexed shapes intersecting with shapes * diff --git a/server/src/main/java/org/elasticsearch/index/query/QueryShardContext.java b/server/src/main/java/org/elasticsearch/index/query/QueryShardContext.java index 865f52c8671b7..79f4398dfebd2 100644 --- a/server/src/main/java/org/elasticsearch/index/query/QueryShardContext.java +++ b/server/src/main/java/org/elasticsearch/index/query/QueryShardContext.java @@ -75,8 +75,8 @@ public class QueryShardContext extends QueryRewriteContext { private static final DeprecationLogger deprecationLogger = new DeprecationLogger( LogManager.getLogger(QueryShardContext.class)); - static final String TYPES_DEPRECATION_MESSAGE = "[types removal] Using the _type field " + - "in queries is deprecated, prefer to filter on a field instead."; + public static final String TYPES_DEPRECATION_MESSAGE = "[types removal] Using the type field " + + "in queries and aggregations is deprecated, prefer to filter on a field instead."; private final ScriptService scriptService; private final IndexSettings indexSettings; diff --git a/server/src/main/java/org/elasticsearch/indices/TermsLookup.java b/server/src/main/java/org/elasticsearch/indices/TermsLookup.java index c1acce072b166..1ab13814f99c9 100644 --- a/server/src/main/java/org/elasticsearch/indices/TermsLookup.java +++ b/server/src/main/java/org/elasticsearch/indices/TermsLookup.java @@ -19,15 +19,17 @@ package org.elasticsearch.indices; +import org.apache.logging.log4j.LogManager; import org.elasticsearch.Version; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; -import org.elasticsearch.common.xcontent.ToXContent.Params; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.xcontent.ToXContentFragment; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.index.query.TermsQueryBuilder; import java.io.IOException; @@ -37,12 +39,31 @@ * Encapsulates the parameters needed to fetch terms. */ public class TermsLookup implements Writeable, ToXContentFragment { + + private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(TermsLookup.class)); + private final String index; - private final String type; + private String type; private final String id; private final String path; private String routing; + public TermsLookup(String index, String id, String path) { + if (id == null) { + throw new IllegalArgumentException("[" + TermsQueryBuilder.NAME + "] query lookup element requires specifying the id."); + } + if (path == null) { + throw new IllegalArgumentException("[" + TermsQueryBuilder.NAME + "] query lookup element requires specifying the path."); + } + if (index == null) { + throw new IllegalArgumentException("[" + TermsQueryBuilder.NAME + "] query lookup element requires specifying the index."); + } + this.index = index; + this.id = id; + this.path = path; + } + + @Deprecated public TermsLookup(String index, String type, String id, String path) { if (id == null) { throw new IllegalArgumentException("[" + TermsQueryBuilder.NAME + "] query lookup element requires specifying the id."); @@ -66,7 +87,11 @@ public TermsLookup(String index, String type, String id, String path) { * Read from a stream. */ public TermsLookup(StreamInput in) throws IOException { - type = in.readString(); + if (in.getVersion().onOrAfter(Version.V_7_0_0)) { + type = in.readOptionalString(); + } else { + type = in.readString(); + } id = in.readString(); path = in.readString(); if (in.getVersion().onOrAfter(Version.V_6_0_0_beta1)) { @@ -82,7 +107,11 @@ public TermsLookup(StreamInput in) throws IOException { @Override public void writeTo(StreamOutput out) throws IOException { - out.writeString(type); + if (out.getVersion().onOrAfter(Version.V_7_0_0)) { + out.writeOptionalString(type); + } else { + out.writeString(type); + } out.writeString(id); out.writeString(path); if (out.getVersion().onOrAfter(Version.V_6_0_0_beta1)) { @@ -97,6 +126,7 @@ public String index() { return index; } + @Deprecated public String type() { return type; } @@ -136,6 +166,7 @@ public static TermsLookup parseTermsLookup(XContentParser parser) throws IOExcep break; case "type": type = parser.text(); + deprecationLogger.deprecatedAndMaybeLog("terms_lookup_with_types", QueryShardContext.TYPES_DEPRECATION_MESSAGE); break; case "id": id = parser.text(); @@ -155,18 +186,28 @@ public static TermsLookup parseTermsLookup(XContentParser parser) throws IOExcep + token + "] after [" + currentFieldName + "]"); } } - return new TermsLookup(index, type, id, path).routing(routing); + if (type == null) { + return new TermsLookup(index, id, path).routing(routing); + } else { + return new TermsLookup(index, type, id, path).routing(routing); + } } @Override public String toString() { - return index + "/" + type + "/" + id + "/" + path; + if (type == null) { + return index + "/" + id + "/" + path; + } else { + return index + "/" + type + "/" + id + "/" + path; + } } @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.field("index", index); - builder.field("type", type); + if (type != null) { + builder.field("type", type); + } builder.field("id", id); builder.field("path", path); if (routing != null) { @@ -177,7 +218,11 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws @Override public int hashCode() { - return Objects.hash(index, type, id, path, routing); + if (type == null) { + return Objects.hash(index, id, path, routing); + } else { + return Objects.hash(index, type, id, path, routing); + } } @Override diff --git a/server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTests.java index 66f955dac7b00..50e5b64b2b9c1 100644 --- a/server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTests.java @@ -38,6 +38,7 @@ import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.index.get.GetResult; +import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.search.internal.SearchContext; import org.elasticsearch.test.AbstractQueryTestCase; import org.elasticsearch.test.VersionUtils; @@ -57,7 +58,6 @@ public class GeoShapeQueryBuilderTests extends AbstractQueryTestCase { protected static String indexedShapeId; - protected static String indexedShapeType; protected static String indexedShapePath; protected static String indexedShapeIndex; protected static String indexedShapeRouting; @@ -94,8 +94,7 @@ protected GeoShapeQueryBuilder doCreateTestQueryBuilder(boolean indexedShape) { } else { indexedShapeToReturn = shape; indexedShapeId = randomAlphaOfLengthBetween(3, 20); - indexedShapeType = randomAlphaOfLengthBetween(3, 20); - builder = new GeoShapeQueryBuilder(fieldName(), indexedShapeId, indexedShapeType); + builder = new GeoShapeQueryBuilder(fieldName(), indexedShapeId); if (randomBoolean()) { indexedShapeIndex = randomAlphaOfLengthBetween(3, 20); builder.indexedShapeIndex(indexedShapeIndex); @@ -128,9 +127,7 @@ protected GeoShapeQueryBuilder doCreateTestQueryBuilder(boolean indexedShape) { protected GetResponse executeGet(GetRequest getRequest) { assertThat(indexedShapeToReturn, notNullValue()); assertThat(indexedShapeId, notNullValue()); - assertThat(indexedShapeType, notNullValue()); assertThat(getRequest.id(), equalTo(indexedShapeId)); - assertThat(getRequest.type(), equalTo(indexedShapeType)); assertThat(getRequest.routing(), equalTo(indexedShapeRouting)); String expectedShapeIndex = indexedShapeIndex == null ? GeoShapeQueryBuilder.DEFAULT_SHAPE_INDEX_NAME : indexedShapeIndex; assertThat(getRequest.index(), equalTo(expectedShapeIndex)); @@ -146,15 +143,14 @@ protected GetResponse executeGet(GetRequest getRequest) { } catch (IOException ex) { throw new ElasticsearchException("boom", ex); } - return new GetResponse(new GetResult(indexedShapeIndex, indexedShapeType, indexedShapeId, 0, 1, 0, true, new BytesArray(json), - null)); + return new GetResponse(new GetResult( + indexedShapeIndex, MapperService.SINGLE_MAPPING_NAME, indexedShapeId, 0, 1, 0, true, new BytesArray(json), null)); } @After public void clearShapeFields() { indexedShapeToReturn = null; indexedShapeId = null; - indexedShapeType = null; indexedShapePath = null; indexedShapeIndex = null; indexedShapeRouting = null; @@ -176,19 +172,13 @@ public void testNoFieldName() throws Exception { } public void testNoShape() throws IOException { - expectThrows(IllegalArgumentException.class, () -> new GeoShapeQueryBuilder(fieldName(), null)); + expectThrows(IllegalArgumentException.class, () -> new GeoShapeQueryBuilder(fieldName(), (ShapeBuilder) null)); } public void testNoIndexedShape() throws IOException { IllegalArgumentException e = expectThrows(IllegalArgumentException.class, - () -> new GeoShapeQueryBuilder(fieldName(), null, "type")); - assertEquals("either shapeBytes or indexedShapeId and indexedShapeType are required", e.getMessage()); - } - - public void testNoIndexedShapeType() throws IOException { - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, - () -> new GeoShapeQueryBuilder(fieldName(), "id", null)); - assertEquals("indexedShapeType is required if indexedShapeId is specified", e.getMessage()); + () -> new GeoShapeQueryBuilder(fieldName(), (String) null)); + assertEquals("either shapeBytes or indexedShapeId is required", e.getMessage()); } public void testNoRelation() throws IOException { diff --git a/server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTypedTests.java b/server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTypedTests.java new file mode 100644 index 0000000000000..622bc575182d3 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTypedTests.java @@ -0,0 +1,233 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.elasticsearch.index.query; + +import org.apache.lucene.search.BooleanQuery; +import org.apache.lucene.search.ConstantScoreQuery; +import org.apache.lucene.search.MatchNoDocsQuery; +import org.apache.lucene.search.Query; +import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.Version; +import org.elasticsearch.action.get.GetRequest; +import org.elasticsearch.action.get.GetResponse; +import org.elasticsearch.cluster.metadata.IndexMetaData; +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.bytes.BytesArray; +import org.elasticsearch.common.geo.ShapeRelation; +import org.elasticsearch.common.geo.builders.ShapeBuilder; +import org.elasticsearch.common.io.stream.BytesStreamOutput; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.index.get.GetResult; +import org.elasticsearch.search.internal.SearchContext; +import org.elasticsearch.test.AbstractQueryTestCase; +import org.elasticsearch.test.VersionUtils; +import org.elasticsearch.test.geo.RandomShapeGenerator; +import org.elasticsearch.test.geo.RandomShapeGenerator.ShapeType; +import org.junit.After; + +import java.io.IOException; + +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.CoreMatchers.notNullValue; +import static org.hamcrest.Matchers.anyOf; +import static org.hamcrest.Matchers.equalTo; + +public class GeoShapeQueryBuilderTypedTests extends AbstractQueryTestCase { + + protected static String indexedShapeId; + protected static String indexedShapeType; + protected static String indexedShapePath; + protected static String indexedShapeIndex; + protected static String indexedShapeRouting; + protected static ShapeBuilder indexedShapeToReturn; + + protected String fieldName() { + return GEO_SHAPE_FIELD_NAME; + } + + @Override + protected Settings createTestIndexSettings() { + // force the new shape impl + Version version = VersionUtils.randomVersionBetween(random(), Version.V_6_6_0, Version.CURRENT); + return Settings.builder() + .put(super.createTestIndexSettings()) + .put(IndexMetaData.SETTING_VERSION_CREATED, version) + .build(); + } + + @Override + protected GeoShapeQueryBuilder doCreateTestQueryBuilder() { + // LatLonShape does not support MultiPoint queries + ShapeType shapeType = randomFrom(ShapeType.POINT, ShapeType.LINESTRING, ShapeType.MULTILINESTRING, ShapeType.POLYGON); + ShapeBuilder shape = RandomShapeGenerator.createShapeWithin(random(), null, shapeType); + GeoShapeQueryBuilder builder; + clearShapeFields(); + + indexedShapeToReturn = shape; + indexedShapeId = randomAlphaOfLengthBetween(3, 20); + indexedShapeType = randomAlphaOfLengthBetween(3, 20); + builder = new GeoShapeQueryBuilder(fieldName(), indexedShapeId, indexedShapeType); + if (randomBoolean()) { + indexedShapeIndex = randomAlphaOfLengthBetween(3, 20); + builder.indexedShapeIndex(indexedShapeIndex); + } + if (randomBoolean()) { + indexedShapePath = randomAlphaOfLengthBetween(3, 20); + builder.indexedShapePath(indexedShapePath); + } + if (randomBoolean()) { + indexedShapeRouting = randomAlphaOfLengthBetween(3, 20); + builder.indexedShapeRouting(indexedShapeRouting); + } + + if (randomBoolean()) { + if (shapeType == ShapeType.LINESTRING || shapeType == ShapeType.MULTILINESTRING) { + builder.relation(randomFrom(ShapeRelation.DISJOINT, ShapeRelation.INTERSECTS)); + } else { + // LatLonShape does not support CONTAINS: + builder.relation(randomFrom(ShapeRelation.DISJOINT, ShapeRelation.INTERSECTS, ShapeRelation.WITHIN)); + } + } + + if (randomBoolean()) { + builder.ignoreUnmapped(randomBoolean()); + } + return builder; + } + + @Override + protected GetResponse executeGet(GetRequest getRequest) { + assertThat(indexedShapeToReturn, notNullValue()); + assertThat(indexedShapeId, notNullValue()); + assertThat(indexedShapeType, notNullValue()); + assertThat(getRequest.id(), equalTo(indexedShapeId)); + assertThat(getRequest.type(), equalTo(indexedShapeType)); + assertThat(getRequest.routing(), equalTo(indexedShapeRouting)); + String expectedShapeIndex = indexedShapeIndex == null ? GeoShapeQueryBuilder.DEFAULT_SHAPE_INDEX_NAME : indexedShapeIndex; + assertThat(getRequest.index(), equalTo(expectedShapeIndex)); + String expectedShapePath = indexedShapePath == null ? GeoShapeQueryBuilder.DEFAULT_SHAPE_FIELD_NAME : indexedShapePath; + String json; + try { + XContentBuilder builder = XContentFactory.jsonBuilder().prettyPrint(); + builder.startObject(); + builder.field(expectedShapePath, indexedShapeToReturn); + builder.field(randomAlphaOfLengthBetween(10, 20), "something"); + builder.endObject(); + json = Strings.toString(builder); + } catch (IOException ex) { + throw new ElasticsearchException("boom", ex); + } + return new GetResponse(new GetResult(indexedShapeIndex, indexedShapeType, indexedShapeId, 0, 1, 0, true, new BytesArray(json), + null)); + } + + @After + public void clearShapeFields() { + indexedShapeToReturn = null; + indexedShapeId = null; + indexedShapeType = null; + indexedShapePath = null; + indexedShapeIndex = null; + indexedShapeRouting = null; + } + + @Override + protected void doAssertLuceneQuery(GeoShapeQueryBuilder queryBuilder, Query query, SearchContext context) throws IOException { + // Logic for doToQuery is complex and is hard to test here. Need to rely + // on Integration tests to determine if created query is correct + // TODO improve GeoShapeQueryBuilder.doToQuery() method to make it + // easier to test here + assertThat(query, anyOf(instanceOf(BooleanQuery.class), instanceOf(ConstantScoreQuery.class))); + } + + @Override + public void testMustRewrite() throws IOException { + GeoShapeQueryBuilder query = doCreateTestQueryBuilder(); + + UnsupportedOperationException e = expectThrows(UnsupportedOperationException.class, () -> query.toQuery(createShardContext())); + assertEquals("query must be rewritten first", e.getMessage()); + QueryBuilder rewrite = rewriteAndFetch(query, createShardContext()); + GeoShapeQueryBuilder geoShapeQueryBuilder = new GeoShapeQueryBuilder(fieldName(), indexedShapeToReturn); + geoShapeQueryBuilder.strategy(query.strategy()); + geoShapeQueryBuilder.relation(query.relation()); + assertEquals(geoShapeQueryBuilder, rewrite); + } + + public void testMultipleRewrite() throws IOException { + GeoShapeQueryBuilder shape = doCreateTestQueryBuilder(); + QueryBuilder builder = new BoolQueryBuilder() + .should(shape) + .should(shape); + + builder = rewriteAndFetch(builder, createShardContext()); + + GeoShapeQueryBuilder expectedShape = new GeoShapeQueryBuilder(fieldName(), indexedShapeToReturn); + expectedShape.strategy(shape.strategy()); + expectedShape.relation(shape.relation()); + QueryBuilder expected = new BoolQueryBuilder() + .should(expectedShape) + .should(expectedShape); + assertEquals(expected, builder); + } + + public void testIgnoreUnmapped() throws IOException { + ShapeType shapeType = ShapeType.randomType(random()); + ShapeBuilder shape = RandomShapeGenerator.createShapeWithin(random(), null, shapeType); + final GeoShapeQueryBuilder queryBuilder = new GeoShapeQueryBuilder("unmapped", shape); + queryBuilder.ignoreUnmapped(true); + Query query = queryBuilder.toQuery(createShardContext()); + assertThat(query, notNullValue()); + assertThat(query, instanceOf(MatchNoDocsQuery.class)); + + final GeoShapeQueryBuilder failingQueryBuilder = new GeoShapeQueryBuilder("unmapped", shape); + failingQueryBuilder.ignoreUnmapped(false); + QueryShardException e = expectThrows(QueryShardException.class, () -> failingQueryBuilder.toQuery(createShardContext())); + assertThat(e.getMessage(), containsString("failed to find geo_shape field [unmapped]")); + } + + public void testSerializationFailsUnlessFetched() throws IOException { + QueryBuilder builder = doCreateTestQueryBuilder(); + QueryBuilder queryBuilder = Rewriteable.rewrite(builder, createShardContext()); + IllegalStateException ise = expectThrows(IllegalStateException.class, () -> queryBuilder.writeTo(new BytesStreamOutput(10))); + assertEquals(ise.getMessage(), "supplier must be null, can't serialize suppliers, missing a rewriteAndFetch?"); + builder = rewriteAndFetch(builder, createShardContext()); + builder.writeTo(new BytesStreamOutput(10)); + } + + @Override + public void testFromXContent() throws IOException { + super.testFromXContent(); + assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); + } + + @Override + public void testUnknownField() throws IOException { + super.testUnknownField(); + assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); + } + + @Override + public void testValidOutput() throws IOException { + super.testValidOutput(); + assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); + } +} diff --git a/server/src/test/java/org/elasticsearch/index/query/IdsQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/IdsQueryBuilderTests.java index c146df73019c2..9c9afd70a2fc6 100644 --- a/server/src/test/java/org/elasticsearch/index/query/IdsQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/IdsQueryBuilderTests.java @@ -19,18 +19,15 @@ package org.elasticsearch.index.query; - import org.apache.lucene.search.MatchNoDocsQuery; import org.apache.lucene.search.Query; import org.apache.lucene.search.TermInSetQuery; -import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.common.ParsingException; import org.elasticsearch.index.mapper.IdFieldMapper; import org.elasticsearch.search.internal.SearchContext; import org.elasticsearch.test.AbstractQueryTestCase; import java.io.IOException; -import java.util.Arrays; import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.Matchers.contains; @@ -40,44 +37,19 @@ public class IdsQueryBuilderTests extends AbstractQueryTestCase @Override protected IdsQueryBuilder doCreateTestQueryBuilder() { - final String type; - if (randomBoolean()) { - if (frequently()) { - type = "_doc"; - } else { - type = randomAlphaOfLengthBetween(1, 10); - } - } else if (randomBoolean()) { - type = MetaData.ALL; - } else { - type = null; - } + IdsQueryBuilder query = new IdsQueryBuilder(); int numberOfIds = randomIntBetween(0, 10); String[] ids = new String[numberOfIds]; for (int i = 0; i < numberOfIds; i++) { ids[i] = randomAlphaOfLengthBetween(1, 10); } - IdsQueryBuilder query; - if (type != null && randomBoolean()) { - query = new IdsQueryBuilder().types(type); - query.addIds(ids); - } else { - query = new IdsQueryBuilder(); - query.addIds(ids); - } + query.addIds(ids); return query; } @Override protected void doAssertLuceneQuery(IdsQueryBuilder queryBuilder, Query query, SearchContext context) throws IOException { - boolean allTypes = queryBuilder.types().length == 0 || - queryBuilder.types().length == 1 && "_all".equals(queryBuilder.types()[0]); - if (queryBuilder.ids().size() == 0 - // no types - || context.getQueryShardContext().fieldMapper(IdFieldMapper.NAME) == null - // there are types, but disjoint from the query - || (allTypes == false && - Arrays.asList(queryBuilder.types()).indexOf(context.mapperService().documentMapper().type()) == -1)) { + if (queryBuilder.ids().size() == 0 || context.getQueryShardContext().fieldMapper(IdFieldMapper.NAME) == null) { assertThat(query, instanceOf(MatchNoDocsQuery.class)); } else { assertThat(query, instanceOf(TermInSetQuery.class)); @@ -85,11 +57,8 @@ protected void doAssertLuceneQuery(IdsQueryBuilder queryBuilder, Query query, Se } public void testIllegalArguments() { - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> new IdsQueryBuilder().types((String[]) null)); - assertEquals("[ids] types cannot be null", e.getMessage()); - IdsQueryBuilder idsQueryBuilder = new IdsQueryBuilder(); - e = expectThrows(IllegalArgumentException.class, () -> idsQueryBuilder.addIds((String[]) null)); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> idsQueryBuilder.addIds((String[]) null)); assertEquals("[ids] ids cannot be null", e.getMessage()); } @@ -104,52 +73,14 @@ public void testFromJson() throws IOException { String json = "{\n" + " \"ids\" : {\n" + - " \"type\" : [ \"my_type\" ],\n" + " \"values\" : [ \"1\", \"100\", \"4\" ],\n" + " \"boost\" : 1.0\n" + " }\n" + "}"; IdsQueryBuilder parsed = (IdsQueryBuilder) parseQuery(json); - checkGeneratedJson(json, parsed); - assertThat(parsed.ids(), contains("1","100","4")); - assertEquals(json, "my_type", parsed.types()[0]); - - // check that type that is not an array and also ids that are numbers are parsed - json = - "{\n" + - " \"ids\" : {\n" + - " \"type\" : \"my_type\",\n" + - " \"values\" : [ 1, 100, 4 ],\n" + - " \"boost\" : 1.0\n" + - " }\n" + - "}"; - parsed = (IdsQueryBuilder) parseQuery(json); - assertThat(parsed.ids(), contains("1","100","4")); - assertEquals(json, "my_type", parsed.types()[0]); - - // check with empty type array - json = - "{\n" + - " \"ids\" : {\n" + - " \"type\" : [ ],\n" + - " \"values\" : [ \"1\", \"100\", \"4\" ],\n" + - " \"boost\" : 1.0\n" + - " }\n" + - "}"; - parsed = (IdsQueryBuilder) parseQuery(json); - assertThat(parsed.ids(), contains("1","100","4")); - assertEquals(json, 0, parsed.types().length); - - // check without type - json = - "{\n" + - " \"ids\" : {\n" + - " \"values\" : [ \"1\", \"100\", \"4\" ],\n" + - " \"boost\" : 1.0\n" + - " }\n" + - "}"; - parsed = (IdsQueryBuilder) parseQuery(json); assertThat(parsed.ids(), contains("1","100","4")); - assertEquals(json, 0, parsed.types().length); + // can't check for {@code checkGeneratedJson(json, parsed)} as + // even if types are null, IdsQueryBuilder uses empty array for decoding them + // which will make {@code checkGeneratedJson(json, parsed)} fail. } } diff --git a/server/src/test/java/org/elasticsearch/index/query/IdsQueryBuilderTypedTests.java b/server/src/test/java/org/elasticsearch/index/query/IdsQueryBuilderTypedTests.java new file mode 100644 index 0000000000000..b268e6ee4a9e8 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/index/query/IdsQueryBuilderTypedTests.java @@ -0,0 +1,192 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.index.query; + + +import org.apache.lucene.search.MatchNoDocsQuery; +import org.apache.lucene.search.Query; +import org.apache.lucene.search.TermInSetQuery; +import org.elasticsearch.cluster.metadata.MetaData; +import org.elasticsearch.common.ParsingException; +import org.elasticsearch.index.mapper.IdFieldMapper; +import org.elasticsearch.search.internal.SearchContext; +import org.elasticsearch.test.AbstractQueryTestCase; + +import java.io.IOException; +import java.util.Arrays; + +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.containsString; + +public class IdsQueryBuilderTypedTests extends AbstractQueryTestCase { + + @Override + protected IdsQueryBuilder doCreateTestQueryBuilder() { + final String type; + if (randomBoolean()) { + if (frequently()) { + type = "_doc"; + } else { + type = randomAlphaOfLengthBetween(1, 10); + } + } else { + type = MetaData.ALL; + } + int numberOfIds = randomIntBetween(0, 10); + String[] ids = new String[numberOfIds]; + for (int i = 0; i < numberOfIds; i++) { + ids[i] = randomAlphaOfLengthBetween(1, 10); + } + IdsQueryBuilder query = new IdsQueryBuilder().types(type); + query.addIds(ids); + return query; + } + + @Override + protected void doAssertLuceneQuery(IdsQueryBuilder queryBuilder, Query query, SearchContext context) throws IOException { + boolean allTypes = queryBuilder.types().length == 0 || + queryBuilder.types().length == 1 && "_all".equals(queryBuilder.types()[0]); + if (queryBuilder.ids().size() == 0 + // no types + || context.getQueryShardContext().fieldMapper(IdFieldMapper.NAME) == null + // there are types, but disjoint from the query + || (allTypes == false && + Arrays.asList(queryBuilder.types()).indexOf(context.mapperService().documentMapper().type()) == -1)) { + assertThat(query, instanceOf(MatchNoDocsQuery.class)); + } else { + assertThat(query, instanceOf(TermInSetQuery.class)); + } + } + + public void testIllegalArguments() { + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> new IdsQueryBuilder().types((String[]) null)); + assertEquals("[ids] types cannot be null", e.getMessage()); + + IdsQueryBuilder idsQueryBuilder = new IdsQueryBuilder(); + e = expectThrows(IllegalArgumentException.class, () -> idsQueryBuilder.addIds((String[]) null)); + assertEquals("[ids] ids cannot be null", e.getMessage()); + } + + // see #7686. + public void testIdsQueryWithInvalidValues() throws Exception { + String query = "{ \"ids\": { \"values\": [[1]] } }"; + ParsingException e = expectThrows(ParsingException.class, () -> parseQuery(query)); + assertThat(e.getMessage(), containsString("[ids] failed to parse field [values]")); + } + + public void testFromJson() throws IOException { + String json = + "{\n" + + " \"ids\" : {\n" + + " \"type\" : [ \"my_type\" ],\n" + + " \"values\" : [ \"1\", \"100\", \"4\" ],\n" + + " \"boost\" : 1.0\n" + + " }\n" + + "}"; + IdsQueryBuilder parsed = (IdsQueryBuilder) parseQuery(json); + checkGeneratedJson(json, parsed); + assertThat(parsed.ids(), contains("1","100","4")); + assertEquals(json, "my_type", parsed.types()[0]); + + // check that type that is not an array and also ids that are numbers are parsed + json = + "{\n" + + " \"ids\" : {\n" + + " \"type\" : \"my_type\",\n" + + " \"values\" : [ 1, 100, 4 ],\n" + + " \"boost\" : 1.0\n" + + " }\n" + + "}"; + parsed = (IdsQueryBuilder) parseQuery(json); + assertThat(parsed.ids(), contains("1","100","4")); + assertEquals(json, "my_type", parsed.types()[0]); + + // check with empty type array + json = + "{\n" + + " \"ids\" : {\n" + + " \"type\" : [ ],\n" + + " \"values\" : [ \"1\", \"100\", \"4\" ],\n" + + " \"boost\" : 1.0\n" + + " }\n" + + "}"; + parsed = (IdsQueryBuilder) parseQuery(json); + assertThat(parsed.ids(), contains("1","100","4")); + assertEquals(json, 0, parsed.types().length); + + assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); + } + + @Override + public void testToQuery() throws IOException { + super.testToQuery(); + assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); + } + + @Override + public void testEqualsAndHashcode() { + super.testEqualsAndHashcode(); + assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); + } + + @Override + public void testUnknownField() throws IOException { + super.testUnknownField(); + assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); + } + + @Override + public void testUnknownObjectException() throws IOException { + super.testUnknownObjectException(); + assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); + } + + @Override + public void testNegativeBoosts() { + super.testNegativeBoosts(); + assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); + } + @Override + public void testMustRewrite() throws IOException { + super.testMustRewrite(); + assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); + } + @Override + public void testQueryWrappedInArray() { + super.testQueryWrappedInArray(); + assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); + } + @Override + public void testValidOutput() throws IOException { + super.testValidOutput(); + assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); + } + @Override + public void testFromXContent() throws IOException { + super.testFromXContent(); + assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); + } + @Override + public void testSerialization() throws IOException { + super.testSerialization(); + assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); + } +} diff --git a/server/src/test/java/org/elasticsearch/index/query/LegacyGeoShapeFieldQueryTests.java b/server/src/test/java/org/elasticsearch/index/query/LegacyGeoShapeFieldQueryTests.java index f549d17977dc1..2dcf3245dfe15 100644 --- a/server/src/test/java/org/elasticsearch/index/query/LegacyGeoShapeFieldQueryTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/LegacyGeoShapeFieldQueryTests.java @@ -59,8 +59,7 @@ protected GeoShapeQueryBuilder doCreateTestQueryBuilder(boolean indexedShape) { } else { indexedShapeToReturn = shape; indexedShapeId = randomAlphaOfLengthBetween(3, 20); - indexedShapeType = randomAlphaOfLengthBetween(3, 20); - builder = new GeoShapeQueryBuilder(fieldName(), indexedShapeId, indexedShapeType); + builder = new GeoShapeQueryBuilder(fieldName(), indexedShapeId); if (randomBoolean()) { indexedShapeIndex = randomAlphaOfLengthBetween(3, 20); builder.indexedShapeIndex(indexedShapeIndex); diff --git a/server/src/test/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilderTests.java index d6a45a165d19b..27dec74173e81 100644 --- a/server/src/test/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilderTests.java @@ -93,13 +93,12 @@ private static String[] randomStringFields() { private Item generateRandomItem() { String index = randomBoolean() ? getIndex().getName() : null; - String type = "doc"; // indexed item or artificial document Item item; if (randomBoolean()) { - item = new Item(index, type, randomAlphaOfLength(10)); + item = new Item(index, randomAlphaOfLength(10)); } else { - item = new Item(index, type, randomArtificialDoc()); + item = new Item(index, randomArtificialDoc()); } // if no field is specified MLT uses all mapped fields for this item if (randomBoolean()) { @@ -345,11 +344,9 @@ public void testFromJson() throws IOException { " \"fields\" : [ \"title\", \"description\" ],\n" + " \"like\" : [ \"and potentially some more text here as well\", {\n" + " \"_index\" : \"imdb\",\n" + - " \"_type\" : \"movies\",\n" + " \"_id\" : \"1\"\n" + " }, {\n" + " \"_index\" : \"imdb\",\n" + - " \"_type\" : \"movies\",\n" + " \"_id\" : \"2\"\n" + " } ],\n" + " \"max_query_terms\" : 12,\n" + diff --git a/server/src/test/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilderTypedTests.java b/server/src/test/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilderTypedTests.java new file mode 100644 index 0000000000000..5c5058ae69678 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilderTypedTests.java @@ -0,0 +1,359 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.index.query; + +import org.apache.lucene.analysis.core.WhitespaceAnalyzer; +import org.apache.lucene.index.Fields; +import org.apache.lucene.index.memory.MemoryIndex; +import org.apache.lucene.search.BooleanClause; +import org.apache.lucene.search.BooleanQuery; +import org.apache.lucene.search.Query; +import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.action.termvectors.MultiTermVectorsItemResponse; +import org.elasticsearch.action.termvectors.MultiTermVectorsRequest; +import org.elasticsearch.action.termvectors.MultiTermVectorsResponse; +import org.elasticsearch.action.termvectors.TermVectorsRequest; +import org.elasticsearch.action.termvectors.TermVectorsResponse; +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.io.stream.BytesStreamOutput; +import org.elasticsearch.common.lucene.search.MoreLikeThisQuery; +import org.elasticsearch.common.xcontent.ToXContent; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.xcontent.json.JsonXContent; +import org.elasticsearch.index.VersionType; +import org.elasticsearch.index.query.MoreLikeThisQueryBuilder.Item; +import org.elasticsearch.search.internal.SearchContext; +import org.elasticsearch.test.AbstractQueryTestCase; +import org.junit.Before; + +import java.io.IOException; +import java.util.Arrays; +import java.util.Collections; +import java.util.EnumSet; +import java.util.HashMap; +import java.util.Map; +import java.util.Set; +import java.util.stream.Stream; + +import static org.elasticsearch.index.query.QueryBuilders.moreLikeThisQuery; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.instanceOf; + +public class MoreLikeThisQueryBuilderTypedTests extends AbstractQueryTestCase { + + private static final String[] SHUFFLE_PROTECTED_FIELDS = new String[]{MoreLikeThisQueryBuilder.DOC.getPreferredName()}; + + private static String[] randomFields; + private static Item[] randomLikeItems; + private static Item[] randomUnlikeItems; + + @Before + public void setup() { + randomFields = randomStringFields(); + randomLikeItems = new Item[randomIntBetween(1, 3)]; + for (int i = 0; i < randomLikeItems.length; i++) { + randomLikeItems[i] = generateRandomItem(); + } + randomUnlikeItems = new Item[randomIntBetween(1, 3)]; + for (int i = 0; i < randomUnlikeItems.length; i++) { + randomUnlikeItems[i] = generateRandomItem(); + } + } + + private static String[] randomStringFields() { + String[] mappedStringFields = new String[]{STRING_FIELD_NAME, STRING_FIELD_NAME_2, STRING_ALIAS_FIELD_NAME}; + String[] unmappedStringFields = generateRandomStringArray(2, 5, false, false); + return Stream.concat(Arrays.stream(mappedStringFields), Arrays.stream(unmappedStringFields)).toArray(String[]::new); + } + + private Item generateRandomItem() { + String index = randomBoolean() ? getIndex().getName() : null; + String type = "doc"; + // indexed item or artificial document + Item item; + if (randomBoolean()) { + item = new Item(index, type, randomAlphaOfLength(10)); + } else { + item = new Item(index, type, randomArtificialDoc()); + } + // if no field is specified MLT uses all mapped fields for this item + if (randomBoolean()) { + item.fields(randomFrom(randomFields)); + } + // per field analyzer + if (randomBoolean()) { + item.perFieldAnalyzer(randomPerFieldAnalyzer()); + } + if (randomBoolean()) { + item.routing(randomAlphaOfLength(10)); + } + if (randomBoolean()) { + item.version(randomInt(5)); + } + if (randomBoolean()) { + item.versionType(randomFrom(VersionType.values())); + } + return item; + } + + private XContentBuilder randomArtificialDoc() { + XContentBuilder doc; + try { + doc = XContentFactory.jsonBuilder().startObject(); + for (String field : randomFields) { + doc.field(field, randomAlphaOfLength(10)); + } + doc.endObject(); + } catch (IOException e) { + throw new ElasticsearchException("Unable to generate random artificial doc!"); + } + return doc; + } + + private Map randomPerFieldAnalyzer() { + Map perFieldAnalyzer = new HashMap<>(); + for (String field : randomFields) { + perFieldAnalyzer.put(field, randomAnalyzer()); + } + return perFieldAnalyzer; + } + + @Override + protected MoreLikeThisQueryBuilder doCreateTestQueryBuilder() { + MoreLikeThisQueryBuilder queryBuilder = new MoreLikeThisQueryBuilder(randomFields, null, randomLikeItems); + if (randomBoolean()) { + queryBuilder.unlike(randomUnlikeItems); + } + if (randomBoolean()) { + queryBuilder.maxQueryTerms(randomInt(25)); + } + if (randomBoolean()) { + queryBuilder.minTermFreq(randomInt(5)); + } + if (randomBoolean()) { + queryBuilder.minDocFreq(randomInt(5)); + } + if (randomBoolean()) { + queryBuilder.maxDocFreq(randomInt(100)); + } + if (randomBoolean()) { + queryBuilder.minWordLength(randomInt(5)); + } + if (randomBoolean()) { + queryBuilder.maxWordLength(randomInt(25)); + } + if (randomBoolean()) { + queryBuilder.stopWords(generateRandomStringArray(5, 5, false, false)); + } + if (randomBoolean()) { + queryBuilder.analyzer(randomAnalyzer()); // fix the analyzer? + } + if (randomBoolean()) { + queryBuilder.minimumShouldMatch(randomMinimumShouldMatch()); + } + if (randomBoolean()) { + queryBuilder.boostTerms(randomFloat() * 10); + } + if (randomBoolean()) { + queryBuilder.include(randomBoolean()); + } + if (randomBoolean()) { + queryBuilder.failOnUnsupportedField(randomBoolean()); + } + return queryBuilder; + } + + /** + * we don't want to shuffle the "doc" field internally in {@link #testFromXContent()} because even though the + * documents would be functionally the same, their {@link BytesReference} representation isn't and thats what we + * compare when check for equality of the original and the shuffled builder + */ + @Override + protected String[] shuffleProtectedFields() { + return SHUFFLE_PROTECTED_FIELDS; + } + + @Override + protected Set getObjectsHoldingArbitraryContent() { + //doc contains arbitrary content, anything can be added to it and no exception will be thrown + return Collections.singleton(MoreLikeThisQueryBuilder.DOC.getPreferredName()); + } + + @Override + protected MultiTermVectorsResponse executeMultiTermVectors(MultiTermVectorsRequest mtvRequest) { + try { + MultiTermVectorsItemResponse[] responses = new MultiTermVectorsItemResponse[mtvRequest.size()]; + int i = 0; + for (TermVectorsRequest request : mtvRequest) { + TermVectorsResponse response = new TermVectorsResponse(request.index(), request.type(), request.id()); + response.setExists(true); + Fields generatedFields; + if (request.doc() != null) { + generatedFields = generateFields(randomFields, request.doc().utf8ToString()); + } else { + generatedFields = + generateFields(request.selectedFields().toArray(new String[request.selectedFields().size()]), request.id()); + } + EnumSet flags = EnumSet.of(TermVectorsRequest.Flag.Positions, TermVectorsRequest.Flag.Offsets); + response.setFields(generatedFields, request.selectedFields(), flags, generatedFields); + responses[i++] = new MultiTermVectorsItemResponse(response, null); + } + return new MultiTermVectorsResponse(responses); + } catch (IOException ex) { + throw new ElasticsearchException("boom", ex); + } + } + + /** + * Here we could go overboard and use a pre-generated indexed random document for a given Item, + * but for now we'd prefer to simply return the id as the content of the document and that for + * every field. + */ + private static Fields generateFields(String[] fieldNames, String text) throws IOException { + MemoryIndex index = new MemoryIndex(); + for (String fieldName : fieldNames) { + index.addField(fieldName, text, new WhitespaceAnalyzer()); + } + return index.createSearcher().getIndexReader().getTermVectors(0); + } + + @Override + protected void doAssertLuceneQuery(MoreLikeThisQueryBuilder queryBuilder, Query query, SearchContext context) throws IOException { + if (queryBuilder.likeItems() != null && queryBuilder.likeItems().length > 0) { + assertThat(query, instanceOf(BooleanQuery.class)); + BooleanQuery booleanQuery = (BooleanQuery) query; + for (BooleanClause booleanClause : booleanQuery) { + if (booleanClause.getQuery() instanceof MoreLikeThisQuery) { + MoreLikeThisQuery moreLikeThisQuery = (MoreLikeThisQuery) booleanClause.getQuery(); + assertThat(moreLikeThisQuery.getLikeFields().length, greaterThan(0)); + } + } + } else { + // we rely on integration tests for a deeper check here + assertThat(query, instanceOf(MoreLikeThisQuery.class)); + } + } + + public void testMoreLikeThisBuilder() throws Exception { + Query parsedQuery = + parseQuery(moreLikeThisQuery(new String[]{"name.first", "name.last"}, new String[]{"something"}, null) + .minTermFreq(1).maxQueryTerms(12)).toQuery(createShardContext()); + assertThat(parsedQuery, instanceOf(MoreLikeThisQuery.class)); + MoreLikeThisQuery mltQuery = (MoreLikeThisQuery) parsedQuery; + assertThat(mltQuery.getMoreLikeFields()[0], equalTo("name.first")); + assertThat(mltQuery.getLikeText(), equalTo("something")); + assertThat(mltQuery.getMinTermFrequency(), equalTo(1)); + assertThat(mltQuery.getMaxQueryTerms(), equalTo(12)); + } + + public void testItemSerialization() throws IOException { + Item expectedItem = generateRandomItem(); + BytesStreamOutput output = new BytesStreamOutput(); + expectedItem.writeTo(output); + Item newItem = new Item(output.bytes().streamInput()); + assertEquals(expectedItem, newItem); + } + + public void testItemCopy() throws IOException { + Item expectedItem = generateRandomItem(); + Item newItem = new Item(expectedItem); + assertEquals(expectedItem, newItem); + } + + public void testItemFromXContent() throws IOException { + Item expectedItem = generateRandomItem(); + String json = Strings.toString(expectedItem.toXContent(XContentFactory.jsonBuilder(), ToXContent.EMPTY_PARAMS)); + XContentParser parser = createParser(JsonXContent.jsonXContent, json); + Item newItem = Item.parse(parser, new Item()); + assertEquals(expectedItem, newItem); + + assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); + } + + @Override + protected boolean isCachable(MoreLikeThisQueryBuilder queryBuilder) { + return queryBuilder.likeItems().length == 0; // items are always fetched + } + + public void testFromJson() throws IOException { + String json = + "{\n" + + " \"more_like_this\" : {\n" + + " \"fields\" : [ \"title\", \"description\" ],\n" + + " \"like\" : [ \"and potentially some more text here as well\", {\n" + + " \"_index\" : \"imdb\",\n" + + " \"_type\" : \"movies\",\n" + + " \"_id\" : \"1\"\n" + + " }, {\n" + + " \"_index\" : \"imdb\",\n" + + " \"_type\" : \"movies\",\n" + + " \"_id\" : \"2\"\n" + + " } ],\n" + + " \"max_query_terms\" : 12,\n" + + " \"min_term_freq\" : 1,\n" + + " \"min_doc_freq\" : 5,\n" + + " \"max_doc_freq\" : 2147483647,\n" + + " \"min_word_length\" : 0,\n" + + " \"max_word_length\" : 0,\n" + + " \"minimum_should_match\" : \"30%\",\n" + + " \"boost_terms\" : 0.0,\n" + + " \"include\" : false,\n" + + " \"fail_on_unsupported_field\" : true,\n" + + " \"boost\" : 1.0\n" + + " }\n" + + "}"; + + MoreLikeThisQueryBuilder parsed = (MoreLikeThisQueryBuilder) parseQuery(json); + checkGeneratedJson(json, parsed); + + assertEquals(json, 2, parsed.fields().length); + assertEquals(json, "and potentially some more text here as well", parsed.likeTexts()[0]); + + assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); + } + + @Override + public void testFromXContent() throws IOException { + super.testFromXContent(); + assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); + } + + @Override + public void testUnknownField() throws IOException { + super.testUnknownField(); + assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); + } + + @Override + public void testValidOutput() throws IOException { + super.testValidOutput(); + assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); + } + + @Override + public void testUnknownObjectException() throws IOException { + super.testUnknownObjectException(); + assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); + } +} diff --git a/server/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java index bc38880651086..dd4e378621270 100644 --- a/server/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java @@ -97,8 +97,11 @@ protected TermsQueryBuilder doCreateTestQueryBuilder() { } private TermsLookup randomTermsLookup() { - return new TermsLookup(randomAlphaOfLength(10), randomAlphaOfLength(10), randomAlphaOfLength(10), - termsPath).routing(randomBoolean() ? randomAlphaOfLength(10) : null); + return new TermsLookup( + randomAlphaOfLength(10), + randomAlphaOfLength(10), + termsPath + ).routing(randomBoolean() ? randomAlphaOfLength(10) : null); } @Override @@ -178,7 +181,6 @@ public void testBothValuesAndLookupSet() throws IOException { " ],\n" + " \"field_lookup\": {\n" + " \"index\": \"pills\",\n" + - " \"type\": \"red\",\n" + " \"id\": \"3\",\n" + " \"path\": \"white rabbit\"\n" + " }\n" + diff --git a/server/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTypedTests.java b/server/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTypedTests.java new file mode 100644 index 0000000000000..2bff1f106403a --- /dev/null +++ b/server/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTypedTests.java @@ -0,0 +1,148 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.index.query; + +import org.apache.lucene.search.BooleanQuery; +import org.apache.lucene.search.ConstantScoreQuery; +import org.apache.lucene.search.MatchNoDocsQuery; +import org.apache.lucene.search.PointInSetQuery; +import org.apache.lucene.search.Query; +import org.apache.lucene.search.TermInSetQuery; +import org.apache.lucene.util.BytesRef; +import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.action.get.GetRequest; +import org.elasticsearch.action.get.GetResponse; +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.bytes.BytesArray; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.index.get.GetResult; +import org.elasticsearch.indices.TermsLookup; +import org.elasticsearch.search.internal.SearchContext; +import org.elasticsearch.test.AbstractQueryTestCase; +import org.junit.Before; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.Objects; +import java.util.stream.Collectors; + +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.either; +import static org.hamcrest.Matchers.instanceOf; + +public class TermsQueryBuilderTypedTests extends AbstractQueryTestCase { + private List randomTerms; + private String termsPath; + + @Before + public void randomTerms() { + List randomTerms = new ArrayList<>(); + String[] strings = generateRandomStringArray(10, 10, false, true); + for (String string : strings) { + randomTerms.add(string); + if (rarely()) { + randomTerms.add(null); + } + } + this.randomTerms = randomTerms; + this.termsPath = randomAlphaOfLength(10).replace('.', '_'); + } + + @Override + protected TermsQueryBuilder doCreateTestQueryBuilder() { + TermsQueryBuilder query = new TermsQueryBuilder( + randomBoolean() ? randomAlphaOfLengthBetween(1,10) : STRING_FIELD_NAME, randomTermsLookup()); + return query; + } + + private TermsLookup randomTermsLookup() { + return new TermsLookup( + randomAlphaOfLength(10), + randomAlphaOfLength(10), + randomAlphaOfLength(10), + termsPath + ).routing(randomBoolean() ? randomAlphaOfLength(10) : null); + } + + @Override + protected void doAssertLuceneQuery(TermsQueryBuilder queryBuilder, Query query, SearchContext context) throws IOException { + if (queryBuilder.termsLookup() != null && randomTerms.size() == 0){ + assertThat(query, instanceOf(MatchNoDocsQuery.class)); + MatchNoDocsQuery matchNoDocsQuery = (MatchNoDocsQuery) query; + assertThat(matchNoDocsQuery.toString(), containsString("No terms supplied for \"terms\" query.")); + } else { + assertThat(query, either(instanceOf(TermInSetQuery.class)) + .or(instanceOf(PointInSetQuery.class)) + .or(instanceOf(ConstantScoreQuery.class))); + if (query instanceof ConstantScoreQuery) { + assertThat(((ConstantScoreQuery) query).getQuery(), instanceOf(BooleanQuery.class)); + } + String fieldName = expectedFieldName(queryBuilder.fieldName()); + TermInSetQuery expected = new TermInSetQuery(fieldName, + randomTerms.stream().filter(Objects::nonNull).map(Object::toString).map(BytesRef::new).collect(Collectors.toList())); + assertEquals(expected, query); + } + } + + @Override + public GetResponse executeGet(GetRequest getRequest) { + String json; + try { + XContentBuilder builder = XContentFactory.jsonBuilder().prettyPrint(); + builder.startObject(); + builder.array(termsPath, randomTerms.toArray(new Object[randomTerms.size()])); + builder.endObject(); + json = Strings.toString(builder); + } catch (IOException ex) { + throw new ElasticsearchException("boom", ex); + } + return new GetResponse(new GetResult(getRequest.index(), getRequest.type(), getRequest.id(), 0, 1, 0, true, + new BytesArray(json), null)); + } + + @Override + public void testMustRewrite() throws IOException { + TermsQueryBuilder termsQueryBuilder = new TermsQueryBuilder(STRING_FIELD_NAME, randomTermsLookup()); + UnsupportedOperationException e = expectThrows(UnsupportedOperationException.class, + () -> termsQueryBuilder.toQuery(createShardContext())); + assertEquals("query must be rewritten first", e.getMessage()); + } + + @Override + public void testFromXContent() throws IOException { + super.testFromXContent(); + assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); + } + + @Override + public void testUnknownField() throws IOException { + super.testUnknownField(); + assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); + } + + @Override + public void testValidOutput() throws IOException { + super.testValidOutput(); + assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); + } +} + diff --git a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java index c90cd3725ecab..01ff8e395a316 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java @@ -163,7 +163,7 @@ public void testUnknownField() throws IOException { * parse exception. Some specific objects do not cause any exception as they can hold arbitrary content; they can be * declared by overriding {@link #getObjectsHoldingArbitraryContent()}. */ - public final void testUnknownObjectException() throws IOException { + public void testUnknownObjectException() throws IOException { Set candidates = new HashSet<>(); // Adds the valid query to the list of queries to modify and test candidates.add(createTestQueryBuilder().toString()); @@ -321,7 +321,7 @@ protected Set getObjectsHoldingArbitraryContent() { * Test that wraps the randomly generated query into an array as follows: { "query_name" : [{}]} * This causes unexpected situations in parser code that may not be handled properly. */ - public final void testQueryWrappedInArray() { + public void testQueryWrappedInArray() { QB queryBuilder = createTestQueryBuilder(); String queryName = queryBuilder.getName(); String validQuery = queryBuilder.toString(); From d7b0cd8a128f59bb90ce9a71e4e63fcf20cb2910 Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Wed, 2 Jan 2019 17:38:29 -0800 Subject: [PATCH 02/14] Some additional refactors and bug fixes. * Add integration test coverage for typeless lookup queries. * Fix a bug around typeless terms lookup queries. * Make sure to provide non-deprecated methods in QueryBuilders. * Make the deprecation messages more accurate. * Update the query DSL documentation. * Avoid creating duplicate query builder tests. Don't use 'ids' when generating random queries to avoid types warnings. --- .../QueryDSLDocumentationTests.java | 2 +- .../query-dsl/geo-shape-query.asciidoc | 4 +- docs/reference/query-dsl/ids-query.asciidoc | 4 - docs/reference/query-dsl/mlt-query.asciidoc | 4 - docs/reference/query-dsl/terms-query.asciidoc | 4 - .../PercolatorFieldMapperTests.java | 3 +- .../test/search/170_terms_query.yml | 4 +- .../search/171_terms_query_with_types.yml | 59 +++ .../index/query/GeoShapeQueryBuilder.java | 14 +- .../index/query/IdsQueryBuilder.java | 20 +- .../index/query/MoreLikeThisQueryBuilder.java | 18 +- .../index/query/QueryBuilders.java | 44 ++- .../index/query/QueryShardContext.java | 4 +- .../index/query/TermsQueryBuilder.java | 18 +- .../elasticsearch/indices/TermsLookup.java | 26 +- .../query/GeoShapeQueryBuilderTests.java | 27 +- .../query/GeoShapeQueryBuilderTypedTests.java | 233 ------------ .../index/query/IdsQueryBuilderTests.java | 96 ++++- .../query/IdsQueryBuilderTypedTests.java | 192 ---------- .../query/MoreLikeThisQueryBuilderTests.java | 25 +- .../MoreLikeThisQueryBuilderTypedTests.java | 359 ------------------ .../index/query/RandomQueryBuilder.java | 3 +- .../index/query/TermsQueryBuilderTests.java | 26 +- .../query/TermsQueryBuilderTypedTests.java | 148 -------- .../search/geo/GeoShapeQueryTests.java | 77 +++- .../search/morelikethis/MoreLikeThisIT.java | 54 ++- .../test/AbstractQueryTestCase.java | 8 +- 27 files changed, 417 insertions(+), 1059 deletions(-) create mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/search/171_terms_query_with_types.yml delete mode 100644 server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTypedTests.java delete mode 100644 server/src/test/java/org/elasticsearch/index/query/IdsQueryBuilderTypedTests.java delete mode 100644 server/src/test/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilderTypedTests.java delete mode 100644 server/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTypedTests.java diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/QueryDSLDocumentationTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/QueryDSLDocumentationTests.java index 3037e4b517eba..cfe9e98f643e6 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/QueryDSLDocumentationTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/QueryDSLDocumentationTests.java @@ -235,7 +235,7 @@ public void testHasParent() { public void testIds() { // tag::ids - idsQuery() + idsQuery() // <1> .addIds("1", "4", "100"); // end::ids } diff --git a/docs/reference/query-dsl/geo-shape-query.asciidoc b/docs/reference/query-dsl/geo-shape-query.asciidoc index f796881d520c6..059d0db14b51a 100644 --- a/docs/reference/query-dsl/geo-shape-query.asciidoc +++ b/docs/reference/query-dsl/geo-shape-query.asciidoc @@ -81,7 +81,7 @@ GET /example/_search ==== Pre-Indexed Shape The Query also supports using a shape which has already been indexed in -another index and/or index type. This is particularly useful for when +another index. This is particularly useful for when you have a pre-defined list of shapes which are useful to your application and you want to reference this using a logical name (for example 'New Zealand') rather than having to provide their coordinates @@ -90,7 +90,6 @@ each time. In this situation it is only necessary to provide: * `id` - The ID of the document that containing the pre-indexed shape. * `index` - Name of the index where the pre-indexed shape is. Defaults to 'shapes'. -* `type` - Index type where the pre-indexed shape is. * `path` - The field specified as path containing the pre-indexed shape. Defaults to 'shape'. * `routing` - The routing of the shape document if required. @@ -130,7 +129,6 @@ GET /example/_search "location": { "indexed_shape": { "index": "shapes", - "type": "_doc", "id": "deu", "path": "location" } diff --git a/docs/reference/query-dsl/ids-query.asciidoc b/docs/reference/query-dsl/ids-query.asciidoc index 55adcb8f94cf8..8798a2fb093f8 100644 --- a/docs/reference/query-dsl/ids-query.asciidoc +++ b/docs/reference/query-dsl/ids-query.asciidoc @@ -10,13 +10,9 @@ GET /_search { "query": { "ids" : { - "type" : "_doc", "values" : ["1", "4", "100"] } } } -------------------------------------------------- // CONSOLE - -The `type` is optional and can be omitted, and can also accept an array -of values. If no type is specified, all types defined in the index mapping are tried. diff --git a/docs/reference/query-dsl/mlt-query.asciidoc b/docs/reference/query-dsl/mlt-query.asciidoc index 19035d96ae04d..64a2a6052df71 100644 --- a/docs/reference/query-dsl/mlt-query.asciidoc +++ b/docs/reference/query-dsl/mlt-query.asciidoc @@ -42,12 +42,10 @@ GET /_search "like" : [ { "_index" : "imdb", - "_type" : "movies", "_id" : "1" }, { "_index" : "imdb", - "_type" : "movies", "_id" : "2" }, "and potentially some more text here as well" @@ -74,7 +72,6 @@ GET /_search "like" : [ { "_index" : "marvel", - "_type" : "quotes", "doc" : { "name": { "first": "Ben", @@ -85,7 +82,6 @@ GET /_search }, { "_index" : "marvel", - "_type" : "quotes", "_id" : "2" } ], diff --git a/docs/reference/query-dsl/terms-query.asciidoc b/docs/reference/query-dsl/terms-query.asciidoc index c0e94900d7d82..db4597fbea504 100644 --- a/docs/reference/query-dsl/terms-query.asciidoc +++ b/docs/reference/query-dsl/terms-query.asciidoc @@ -36,9 +36,6 @@ The terms lookup mechanism supports the following options: `index`:: The index to fetch the term values from. -`type`:: - The type to fetch the term values from. - `id`:: The id of the document to fetch the term values from. @@ -93,7 +90,6 @@ GET /tweets/_search "terms" : { "user" : { "index" : "users", - "type" : "_doc", "id" : "2", "path" : "followers" } diff --git a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorFieldMapperTests.java b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorFieldMapperTests.java index 001a3d959858f..f1747d1977561 100644 --- a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorFieldMapperTests.java +++ b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorFieldMapperTests.java @@ -537,7 +537,7 @@ public void testStoringQueries() throws Exception { public void testQueryWithRewrite() throws Exception { addQueryFieldMappings(); client().prepareIndex("remote", "doc", "1").setSource("field", "value").get(); - QueryBuilder queryBuilder = termsLookupQuery("field", new TermsLookup("remote", "doc", "1", "field")); + QueryBuilder queryBuilder = termsLookupQuery("field", new TermsLookup("remote", "1", "field")); ParsedDocument doc = mapperService.documentMapper("doc").parse(new SourceToParse("test", "doc", "1", BytesReference.bytes(XContentFactory .jsonBuilder() @@ -553,7 +553,6 @@ public void testQueryWithRewrite() throws Exception { PlainActionFuture future = new PlainActionFuture<>(); Rewriteable.rewriteAndFetch(queryBuilder, shardContext, future); assertQueryBuilder(qbSource, future.get()); - assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/170_terms_query.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/170_terms_query.yml index 515dcfe463069..3966a6a182a62 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/search/170_terms_query.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search/170_terms_query.yml @@ -48,7 +48,7 @@ search: rest_total_hits_as_int: true index: test_index - body: {"query" : {"terms" : {"user" : {"index" : "test_index", "type" : "test_type", "id" : "u1", "path" : "followers"}}}} + body: {"query" : {"terms" : {"user" : {"index" : "test_index", "id" : "u1", "path" : "followers"}}}} - match: { hits.total: 2 } - do: @@ -56,4 +56,4 @@ search: rest_total_hits_as_int: true index: test_index - body: {"query" : {"terms" : {"user" : {"index" : "test_index", "type" : "test_type", "id" : "u2", "path" : "followers"}}}} + body: {"query" : {"terms" : {"user" : {"index" : "test_index", "id" : "u2", "path" : "followers"}}}} diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/171_terms_query_with_types.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/171_terms_query_with_types.yml new file mode 100644 index 0000000000000..515dcfe463069 --- /dev/null +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search/171_terms_query_with_types.yml @@ -0,0 +1,59 @@ +--- +"Terms Query with No.of terms exceeding index.max_terms_count should FAIL": + - skip: + version: " - 6.99.99" + reason: index.max_terms_count setting has been added in 7.0.0 + - do: + indices.create: + index: test_index + body: + settings: + number_of_shards: 1 + index.max_terms_count: 2 + mappings: + test_type: + properties: + user: + type: keyword + followers: + type: keyword + - do: + bulk: + refresh: true + body: + - '{"index": {"_index": "test_index", "_type": "test_type", "_id": "u1"}}' + - '{"user": "u1", "followers": ["u2", "u3"]}' + - '{"index": {"_index": "test_index", "_type": "test_type", "_id": "u2"}}' + - '{"user": "u2", "followers": ["u1", "u3", "u4"]}' + - '{"index": {"_index": "test_index", "_type": "test_type", "_id": "u3"}}' + - '{"user": "u3", "followers": ["u1"]}' + - '{"index": {"_index": "test_index", "_type": "test_type", "_id": "u4"}}' + - '{"user": "u4", "followers": ["u3"]}' + + - do: + search: + rest_total_hits_as_int: true + index: test_index + body: {"query" : {"terms" : {"user" : ["u1", "u2"]}}} + - match: { hits.total: 2 } + + - do: + catch: bad_request + search: + rest_total_hits_as_int: true + index: test_index + body: {"query" : {"terms" : {"user" : ["u1", "u2", "u3"]}}} + + - do: + search: + rest_total_hits_as_int: true + index: test_index + body: {"query" : {"terms" : {"user" : {"index" : "test_index", "type" : "test_type", "id" : "u1", "path" : "followers"}}}} + - match: { hits.total: 2 } + + - do: + catch: bad_request + search: + rest_total_hits_as_int: true + index: test_index + body: {"query" : {"terms" : {"user" : {"index" : "test_index", "type" : "test_type", "id" : "u2", "path" : "followers"}}}} diff --git a/server/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java index 0de62f361c7d3..e9837668a09aa 100644 --- a/server/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java @@ -19,11 +19,11 @@ package org.elasticsearch.index.query; +import org.apache.logging.log4j.LogManager; import org.apache.lucene.document.LatLonShape; import org.apache.lucene.geo.Line; import org.apache.lucene.geo.Polygon; import org.apache.lucene.geo.Rectangle; -import org.apache.logging.log4j.LogManager; import org.apache.lucene.search.BooleanClause; import org.apache.lucene.search.BooleanQuery; import org.apache.lucene.search.ConstantScoreQuery; @@ -69,7 +69,10 @@ */ public class GeoShapeQueryBuilder extends AbstractQueryBuilder { public static final String NAME = "geo_shape"; - private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(GeoShapeQueryBuilder.class)); + private static final DeprecationLogger deprecationLogger = new DeprecationLogger( + LogManager.getLogger(GeoShapeQueryBuilder.class)); + 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"; @@ -664,8 +667,6 @@ public static GeoShapeQueryBuilder fromXContent(XContentParser parser) throws IO if (SHAPE_ID_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { id = parser.text(); } else if (SHAPE_TYPE_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { - deprecationLogger.deprecatedAndMaybeLog( - "geo_share_query_with_types", QueryShardContext.TYPES_DEPRECATION_MESSAGE); type = parser.text(); } else if (SHAPE_INDEX_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { index = parser.text(); @@ -699,6 +700,11 @@ public static GeoShapeQueryBuilder fromXContent(XContentParser parser) throws IO } } GeoShapeQueryBuilder builder; + if (type != null) { + deprecationLogger.deprecatedAndMaybeLog( + "geo_share_query_with_types", TYPES_DEPRECATION_MESSAGE); + } + if (shape != null) { builder = new GeoShapeQueryBuilder(fieldName, shape); } else { diff --git a/server/src/main/java/org/elasticsearch/index/query/IdsQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/IdsQueryBuilder.java index e4e79ffa2b144..c42ca72163b8e 100644 --- a/server/src/main/java/org/elasticsearch/index/query/IdsQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/IdsQueryBuilder.java @@ -53,7 +53,9 @@ */ public class IdsQueryBuilder extends AbstractQueryBuilder { public static final String NAME = "ids"; - private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(IdsQueryBuilder.class)); + private static final DeprecationLogger deprecationLogger = new DeprecationLogger( + LogManager.getLogger(IdsQueryBuilder.class)); + static final String TYPES_DEPRECATION_MESSAGE = "[types removal] Types are deprecated in [ids] queries."; private static final ParseField TYPE_FIELD = new ParseField("type"); private static final ParseField VALUES_FIELD = new ParseField("values"); @@ -87,17 +89,11 @@ protected void doWriteTo(StreamOutput out) throws IOException { /** * Add types to query */ - // TODO: Remove in 8.0 @Deprecated public IdsQueryBuilder types(String... types) { if (types == null) { throw new IllegalArgumentException("[" + NAME + "] types cannot be null"); } - // Even if types are null, IdsQueryBuilder uses an empty array for decoding types. - // For this reason, issue deprecation warning if types contain something. - if (types.length > 0) { - deprecationLogger.deprecatedAndMaybeLog("ids_query_with_types", QueryShardContext.TYPES_DEPRECATION_MESSAGE); - } this.types = types; return this; } @@ -131,7 +127,9 @@ public Set ids() { @Override protected void doXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(NAME); - builder.array(TYPE_FIELD.getPreferredName(), types); + if (types.length > 0) { + builder.array(TYPE_FIELD.getPreferredName(), types); + } builder.startArray(VALUES_FIELD.getPreferredName()); for (String value : ids) { builder.value(value); @@ -152,7 +150,11 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep public static IdsQueryBuilder fromXContent(XContentParser parser) { try { - return PARSER.apply(parser, null); + IdsQueryBuilder builder = PARSER.apply(parser, null); + if (builder.types().length > 0) { + deprecationLogger.deprecatedAndMaybeLog("ids_query_with_types", TYPES_DEPRECATION_MESSAGE); + } + return builder; } catch (IllegalArgumentException e) { throw new ParsingException(parser.getTokenLocation(), e.getMessage(), e); } diff --git a/server/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java index 3e07d0efd7dfc..aada4e982f3d4 100644 --- a/server/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java @@ -68,6 +68,7 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; +import java.util.stream.Stream; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; @@ -78,7 +79,10 @@ */ public class MoreLikeThisQueryBuilder extends AbstractQueryBuilder { public static final String NAME = "more_like_this"; - private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(MoreLikeThisQueryBuilder.class)); + private static final DeprecationLogger deprecationLogger = new DeprecationLogger( + LogManager.getLogger(MoreLikeThisQueryBuilder.class)); + static final String TYPES_DEPRECATION_MESSAGE = "[types removal] Types are deprecated in [more_like_this] " + + "queries. The type should no longer be specified in the [like] and [unlike] sections."; public static final int DEFAULT_MAX_QUERY_TERMS = XMoreLikeThis.DEFAULT_MAX_QUERY_TERMS; @@ -182,7 +186,6 @@ public Item() { this.versionType = copy.versionType; } - /** * Constructor for a given item / document request * @@ -398,8 +401,6 @@ public static Item parse(XContentParser parser, Item item) throws IOException { if (INDEX.match(currentFieldName, parser.getDeprecationHandler())) { item.index = parser.text(); } else if (TYPE.match(currentFieldName, parser.getDeprecationHandler())) { - deprecationLogger.deprecatedAndMaybeLog( - "more_like_this_query_with_types", QueryShardContext.TYPES_DEPRECATION_MESSAGE); item.type = parser.text(); } else if (ID.match(currentFieldName, parser.getDeprecationHandler())) { item.id = parser.text(); @@ -950,9 +951,18 @@ public static MoreLikeThisQueryBuilder fromXContent(XContentParser parser) throw if (stopWords != null) { moreLikeThisQueryBuilder.stopWords(stopWords); } + + if (!moreLikeThisQueryBuilder.isTypeless()) { + deprecationLogger.deprecatedAndMaybeLog("more_like_this_query_with_types", TYPES_DEPRECATION_MESSAGE); + } return moreLikeThisQueryBuilder; } + public boolean isTypeless() { + return Stream.concat(Arrays.stream(likeItems), Arrays.stream(unlikeItems)) + .allMatch(item -> item.type == null); + } + private static void parseLikeField(XContentParser parser, List texts, List items) throws IOException { if (parser.currentToken().isValue()) { texts.add(parser.text()); diff --git a/server/src/main/java/org/elasticsearch/index/query/QueryBuilders.java b/server/src/main/java/org/elasticsearch/index/query/QueryBuilders.java index 28f9cce16eab0..5ac70781286a4 100644 --- a/server/src/main/java/org/elasticsearch/index/query/QueryBuilders.java +++ b/server/src/main/java/org/elasticsearch/index/query/QueryBuilders.java @@ -121,6 +121,8 @@ public static IdsQueryBuilder idsQuery() { * Constructs a query that will match only specific ids within types. * * @param types The mapping/doc type + * + * @deprecated Types are in the process of being removed, use {@link #idsQuery()} instead. */ @Deprecated public static IdsQueryBuilder idsQuery(String... types) { @@ -647,14 +649,18 @@ public static GeoShapeQueryBuilder geoShapeQuery(String name, ShapeBuilder shape return new GeoShapeQueryBuilder(name, shape); } - public static GeoShapeQueryBuilder geoShapeQuery(String name, String indexedShapeId, String indexedShapeType) { - return new GeoShapeQueryBuilder(name, indexedShapeId, indexedShapeType); - } - public static GeoShapeQueryBuilder geoShapeQuery(String name, String indexedShapeId) { 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 * @@ -667,6 +673,16 @@ public static GeoShapeQueryBuilder geoIntersectionQuery(String name, ShapeBuilde return builder; } + public static GeoShapeQueryBuilder geoIntersectionQuery(String name, String indexedShapeId) { + GeoShapeQueryBuilder builder = geoShapeQuery(name, indexedShapeId); + builder.relation(ShapeRelation.INTERSECTS); + 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); @@ -685,6 +701,16 @@ public static GeoShapeQueryBuilder geoWithinQuery(String name, ShapeBuilder shap return builder; } + public static GeoShapeQueryBuilder geoWithinQuery(String name, String indexedShapeId) { + GeoShapeQueryBuilder builder = geoShapeQuery(name, indexedShapeId); + builder.relation(ShapeRelation.WITHIN); + 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); @@ -703,6 +729,16 @@ public static GeoShapeQueryBuilder geoDisjointQuery(String name, ShapeBuilder sh return builder; } + public static GeoShapeQueryBuilder geoDisjointQuery(String name, String indexedShapeId) { + GeoShapeQueryBuilder builder = geoShapeQuery(name, indexedShapeId); + builder.relation(ShapeRelation.DISJOINT); + 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); diff --git a/server/src/main/java/org/elasticsearch/index/query/QueryShardContext.java b/server/src/main/java/org/elasticsearch/index/query/QueryShardContext.java index 79f4398dfebd2..865f52c8671b7 100644 --- a/server/src/main/java/org/elasticsearch/index/query/QueryShardContext.java +++ b/server/src/main/java/org/elasticsearch/index/query/QueryShardContext.java @@ -75,8 +75,8 @@ public class QueryShardContext extends QueryRewriteContext { private static final DeprecationLogger deprecationLogger = new DeprecationLogger( LogManager.getLogger(QueryShardContext.class)); - public static final String TYPES_DEPRECATION_MESSAGE = "[types removal] Using the type field " + - "in queries and aggregations is deprecated, prefer to filter on a field instead."; + static final String TYPES_DEPRECATION_MESSAGE = "[types removal] Using the _type field " + + "in queries is deprecated, prefer to filter on a field instead."; private final ScriptService scriptService; private final IndexSettings indexSettings; diff --git a/server/src/main/java/org/elasticsearch/index/query/TermsQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/TermsQueryBuilder.java index a144cbf088a01..ee3192f02e0ae 100644 --- a/server/src/main/java/org/elasticsearch/index/query/TermsQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/TermsQueryBuilder.java @@ -19,6 +19,7 @@ package org.elasticsearch.index.query; +import org.apache.logging.log4j.LogManager; import org.apache.lucene.search.Query; import org.apache.lucene.search.TermInSetQuery; import org.apache.lucene.util.BytesRef; @@ -34,6 +35,7 @@ import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.lucene.BytesRefs; import org.elasticsearch.common.lucene.search.Queries; import org.elasticsearch.common.xcontent.XContentBuilder; @@ -63,6 +65,11 @@ public class TermsQueryBuilder extends AbstractQueryBuilder { public static final String NAME = "terms"; + private static final DeprecationLogger deprecationLogger = new DeprecationLogger( + LogManager.getLogger(TermsQueryBuilder.class)); + static final String TYPES_DEPRECATION_MESSAGE = "[types removal] Types are deprecated " + + "in [terms] lookup queries."; + private final String fieldName; private final List values; private final TermsLookup termsLookup; @@ -391,6 +398,11 @@ public static TermsQueryBuilder fromXContent(XContentParser parser) throws IOExc throw new ParsingException(parser.getTokenLocation(), "[" + TermsQueryBuilder.NAME + "] query requires a field name, " + "followed by array of terms or a document lookup specification"); } + + if (termsLookup != null && termsLookup.type() != null) { + deprecationLogger.deprecatedAndMaybeLog("terms_lookup_with_types", TYPES_DEPRECATION_MESSAGE); + } + return new TermsQueryBuilder(fieldName, values, termsLookup) .boost(boost) .queryName(queryName); @@ -442,8 +454,10 @@ protected Query doToQuery(QueryShardContext context) throws IOException { } private void fetch(TermsLookup termsLookup, Client client, ActionListener> actionListener) { - GetRequest getRequest = new GetRequest(termsLookup.index(), termsLookup.type(), termsLookup.id()) - .preference("_local").routing(termsLookup.routing()); + GetRequest getRequest = termsLookup.type() == null + ? new GetRequest(termsLookup.index(), termsLookup.id()) + : new GetRequest(termsLookup.index(), termsLookup.type(), termsLookup.id()); + getRequest.preference("_local").routing(termsLookup.routing()); client.get(getRequest, new ActionListener() { @Override public void onResponse(GetResponse getResponse) { diff --git a/server/src/main/java/org/elasticsearch/indices/TermsLookup.java b/server/src/main/java/org/elasticsearch/indices/TermsLookup.java index 1ab13814f99c9..195e0401b2f64 100644 --- a/server/src/main/java/org/elasticsearch/indices/TermsLookup.java +++ b/server/src/main/java/org/elasticsearch/indices/TermsLookup.java @@ -19,17 +19,15 @@ package org.elasticsearch.indices; -import org.apache.logging.log4j.LogManager; import org.elasticsearch.Version; +import org.elasticsearch.common.Nullable; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; -import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.xcontent.ToXContentFragment; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.index.query.TermsQueryBuilder; import java.io.IOException; @@ -39,28 +37,14 @@ * Encapsulates the parameters needed to fetch terms. */ public class TermsLookup implements Writeable, ToXContentFragment { - - private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(TermsLookup.class)); - private final String index; - private String type; + private @Nullable String type; private final String id; private final String path; private String routing; public TermsLookup(String index, String id, String path) { - if (id == null) { - throw new IllegalArgumentException("[" + TermsQueryBuilder.NAME + "] query lookup element requires specifying the id."); - } - if (path == null) { - throw new IllegalArgumentException("[" + TermsQueryBuilder.NAME + "] query lookup element requires specifying the path."); - } - if (index == null) { - throw new IllegalArgumentException("[" + TermsQueryBuilder.NAME + "] query lookup element requires specifying the index."); - } - this.index = index; - this.id = id; - this.path = path; + this(index, null, id, path); } @Deprecated @@ -68,9 +52,6 @@ public TermsLookup(String index, String type, String id, String path) { if (id == null) { throw new IllegalArgumentException("[" + TermsQueryBuilder.NAME + "] query lookup element requires specifying the id."); } - if (type == null) { - throw new IllegalArgumentException("[" + TermsQueryBuilder.NAME + "] query lookup element requires specifying the type."); - } if (path == null) { throw new IllegalArgumentException("[" + TermsQueryBuilder.NAME + "] query lookup element requires specifying the path."); } @@ -166,7 +147,6 @@ public static TermsLookup parseTermsLookup(XContentParser parser) throws IOExcep break; case "type": type = parser.text(); - deprecationLogger.deprecatedAndMaybeLog("terms_lookup_with_types", QueryShardContext.TYPES_DEPRECATION_MESSAGE); break; case "id": id = parser.text(); diff --git a/server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTests.java index 50e5b64b2b9c1..13a5594c49366 100644 --- a/server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTests.java @@ -36,6 +36,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.index.get.GetResult; import org.elasticsearch.index.mapper.MapperService; @@ -58,6 +59,7 @@ public class GeoShapeQueryBuilderTests extends AbstractQueryTestCase { protected static String indexedShapeId; + protected static String indexedShapeType; protected static String indexedShapePath; protected static String indexedShapeIndex; protected static String indexedShapeRouting; @@ -94,7 +96,8 @@ protected GeoShapeQueryBuilder doCreateTestQueryBuilder(boolean indexedShape) { } else { indexedShapeToReturn = shape; indexedShapeId = randomAlphaOfLengthBetween(3, 20); - builder = new GeoShapeQueryBuilder(fieldName(), indexedShapeId); + indexedShapeType = randomAlphaOfLengthBetween(3, 20); + builder = new GeoShapeQueryBuilder(fieldName(), indexedShapeId, indexedShapeType); if (randomBoolean()) { indexedShapeIndex = randomAlphaOfLengthBetween(3, 20); builder.indexedShapeIndex(indexedShapeIndex); @@ -125,9 +128,12 @@ protected GeoShapeQueryBuilder doCreateTestQueryBuilder(boolean indexedShape) { @Override protected GetResponse executeGet(GetRequest getRequest) { + String indexedType = indexedShapeType != null ? indexedShapeType : MapperService.SINGLE_MAPPING_NAME; + assertThat(indexedShapeToReturn, notNullValue()); assertThat(indexedShapeId, notNullValue()); assertThat(getRequest.id(), equalTo(indexedShapeId)); + assertThat(getRequest.type(), equalTo(indexedType)); assertThat(getRequest.routing(), equalTo(indexedShapeRouting)); String expectedShapeIndex = indexedShapeIndex == null ? GeoShapeQueryBuilder.DEFAULT_SHAPE_INDEX_NAME : indexedShapeIndex; assertThat(getRequest.index(), equalTo(expectedShapeIndex)); @@ -143,14 +149,15 @@ protected GetResponse executeGet(GetRequest getRequest) { } catch (IOException ex) { throw new ElasticsearchException("boom", ex); } - return new GetResponse(new GetResult( - indexedShapeIndex, MapperService.SINGLE_MAPPING_NAME, indexedShapeId, 0, 1, 0, true, new BytesArray(json), null)); + return new GetResponse(new GetResult(indexedShapeIndex, indexedType, indexedShapeId, 0, 1, 0, true, new BytesArray(json), + null)); } @After public void clearShapeFields() { indexedShapeToReturn = null; indexedShapeId = null; + indexedShapeType = null; indexedShapePath = null; indexedShapeIndex = null; indexedShapeRouting = null; @@ -177,7 +184,7 @@ public void testNoShape() throws IOException { public void testNoIndexedShape() throws IOException { IllegalArgumentException e = expectThrows(IllegalArgumentException.class, - () -> new GeoShapeQueryBuilder(fieldName(), (String) null)); + () -> new GeoShapeQueryBuilder(fieldName(), null, "type")); assertEquals("either shapeBytes or indexedShapeId is required", e.getMessage()); } @@ -276,4 +283,16 @@ public void testSerializationFailsUnlessFetched() throws IOException { builder = rewriteAndFetch(builder, createShardContext()); builder.writeTo(new BytesStreamOutput(10)); } + + @Override + 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; + } } diff --git a/server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTypedTests.java b/server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTypedTests.java deleted file mode 100644 index 622bc575182d3..0000000000000 --- a/server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTypedTests.java +++ /dev/null @@ -1,233 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.elasticsearch.index.query; - -import org.apache.lucene.search.BooleanQuery; -import org.apache.lucene.search.ConstantScoreQuery; -import org.apache.lucene.search.MatchNoDocsQuery; -import org.apache.lucene.search.Query; -import org.elasticsearch.ElasticsearchException; -import org.elasticsearch.Version; -import org.elasticsearch.action.get.GetRequest; -import org.elasticsearch.action.get.GetResponse; -import org.elasticsearch.cluster.metadata.IndexMetaData; -import org.elasticsearch.common.Strings; -import org.elasticsearch.common.bytes.BytesArray; -import org.elasticsearch.common.geo.ShapeRelation; -import org.elasticsearch.common.geo.builders.ShapeBuilder; -import org.elasticsearch.common.io.stream.BytesStreamOutput; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.common.xcontent.XContentFactory; -import org.elasticsearch.index.get.GetResult; -import org.elasticsearch.search.internal.SearchContext; -import org.elasticsearch.test.AbstractQueryTestCase; -import org.elasticsearch.test.VersionUtils; -import org.elasticsearch.test.geo.RandomShapeGenerator; -import org.elasticsearch.test.geo.RandomShapeGenerator.ShapeType; -import org.junit.After; - -import java.io.IOException; - -import static org.hamcrest.CoreMatchers.containsString; -import static org.hamcrest.CoreMatchers.instanceOf; -import static org.hamcrest.CoreMatchers.notNullValue; -import static org.hamcrest.Matchers.anyOf; -import static org.hamcrest.Matchers.equalTo; - -public class GeoShapeQueryBuilderTypedTests extends AbstractQueryTestCase { - - protected static String indexedShapeId; - protected static String indexedShapeType; - protected static String indexedShapePath; - protected static String indexedShapeIndex; - protected static String indexedShapeRouting; - protected static ShapeBuilder indexedShapeToReturn; - - protected String fieldName() { - return GEO_SHAPE_FIELD_NAME; - } - - @Override - protected Settings createTestIndexSettings() { - // force the new shape impl - Version version = VersionUtils.randomVersionBetween(random(), Version.V_6_6_0, Version.CURRENT); - return Settings.builder() - .put(super.createTestIndexSettings()) - .put(IndexMetaData.SETTING_VERSION_CREATED, version) - .build(); - } - - @Override - protected GeoShapeQueryBuilder doCreateTestQueryBuilder() { - // LatLonShape does not support MultiPoint queries - ShapeType shapeType = randomFrom(ShapeType.POINT, ShapeType.LINESTRING, ShapeType.MULTILINESTRING, ShapeType.POLYGON); - ShapeBuilder shape = RandomShapeGenerator.createShapeWithin(random(), null, shapeType); - GeoShapeQueryBuilder builder; - clearShapeFields(); - - indexedShapeToReturn = shape; - indexedShapeId = randomAlphaOfLengthBetween(3, 20); - indexedShapeType = randomAlphaOfLengthBetween(3, 20); - builder = new GeoShapeQueryBuilder(fieldName(), indexedShapeId, indexedShapeType); - if (randomBoolean()) { - indexedShapeIndex = randomAlphaOfLengthBetween(3, 20); - builder.indexedShapeIndex(indexedShapeIndex); - } - if (randomBoolean()) { - indexedShapePath = randomAlphaOfLengthBetween(3, 20); - builder.indexedShapePath(indexedShapePath); - } - if (randomBoolean()) { - indexedShapeRouting = randomAlphaOfLengthBetween(3, 20); - builder.indexedShapeRouting(indexedShapeRouting); - } - - if (randomBoolean()) { - if (shapeType == ShapeType.LINESTRING || shapeType == ShapeType.MULTILINESTRING) { - builder.relation(randomFrom(ShapeRelation.DISJOINT, ShapeRelation.INTERSECTS)); - } else { - // LatLonShape does not support CONTAINS: - builder.relation(randomFrom(ShapeRelation.DISJOINT, ShapeRelation.INTERSECTS, ShapeRelation.WITHIN)); - } - } - - if (randomBoolean()) { - builder.ignoreUnmapped(randomBoolean()); - } - return builder; - } - - @Override - protected GetResponse executeGet(GetRequest getRequest) { - assertThat(indexedShapeToReturn, notNullValue()); - assertThat(indexedShapeId, notNullValue()); - assertThat(indexedShapeType, notNullValue()); - assertThat(getRequest.id(), equalTo(indexedShapeId)); - assertThat(getRequest.type(), equalTo(indexedShapeType)); - assertThat(getRequest.routing(), equalTo(indexedShapeRouting)); - String expectedShapeIndex = indexedShapeIndex == null ? GeoShapeQueryBuilder.DEFAULT_SHAPE_INDEX_NAME : indexedShapeIndex; - assertThat(getRequest.index(), equalTo(expectedShapeIndex)); - String expectedShapePath = indexedShapePath == null ? GeoShapeQueryBuilder.DEFAULT_SHAPE_FIELD_NAME : indexedShapePath; - String json; - try { - XContentBuilder builder = XContentFactory.jsonBuilder().prettyPrint(); - builder.startObject(); - builder.field(expectedShapePath, indexedShapeToReturn); - builder.field(randomAlphaOfLengthBetween(10, 20), "something"); - builder.endObject(); - json = Strings.toString(builder); - } catch (IOException ex) { - throw new ElasticsearchException("boom", ex); - } - return new GetResponse(new GetResult(indexedShapeIndex, indexedShapeType, indexedShapeId, 0, 1, 0, true, new BytesArray(json), - null)); - } - - @After - public void clearShapeFields() { - indexedShapeToReturn = null; - indexedShapeId = null; - indexedShapeType = null; - indexedShapePath = null; - indexedShapeIndex = null; - indexedShapeRouting = null; - } - - @Override - protected void doAssertLuceneQuery(GeoShapeQueryBuilder queryBuilder, Query query, SearchContext context) throws IOException { - // Logic for doToQuery is complex and is hard to test here. Need to rely - // on Integration tests to determine if created query is correct - // TODO improve GeoShapeQueryBuilder.doToQuery() method to make it - // easier to test here - assertThat(query, anyOf(instanceOf(BooleanQuery.class), instanceOf(ConstantScoreQuery.class))); - } - - @Override - public void testMustRewrite() throws IOException { - GeoShapeQueryBuilder query = doCreateTestQueryBuilder(); - - UnsupportedOperationException e = expectThrows(UnsupportedOperationException.class, () -> query.toQuery(createShardContext())); - assertEquals("query must be rewritten first", e.getMessage()); - QueryBuilder rewrite = rewriteAndFetch(query, createShardContext()); - GeoShapeQueryBuilder geoShapeQueryBuilder = new GeoShapeQueryBuilder(fieldName(), indexedShapeToReturn); - geoShapeQueryBuilder.strategy(query.strategy()); - geoShapeQueryBuilder.relation(query.relation()); - assertEquals(geoShapeQueryBuilder, rewrite); - } - - public void testMultipleRewrite() throws IOException { - GeoShapeQueryBuilder shape = doCreateTestQueryBuilder(); - QueryBuilder builder = new BoolQueryBuilder() - .should(shape) - .should(shape); - - builder = rewriteAndFetch(builder, createShardContext()); - - GeoShapeQueryBuilder expectedShape = new GeoShapeQueryBuilder(fieldName(), indexedShapeToReturn); - expectedShape.strategy(shape.strategy()); - expectedShape.relation(shape.relation()); - QueryBuilder expected = new BoolQueryBuilder() - .should(expectedShape) - .should(expectedShape); - assertEquals(expected, builder); - } - - public void testIgnoreUnmapped() throws IOException { - ShapeType shapeType = ShapeType.randomType(random()); - ShapeBuilder shape = RandomShapeGenerator.createShapeWithin(random(), null, shapeType); - final GeoShapeQueryBuilder queryBuilder = new GeoShapeQueryBuilder("unmapped", shape); - queryBuilder.ignoreUnmapped(true); - Query query = queryBuilder.toQuery(createShardContext()); - assertThat(query, notNullValue()); - assertThat(query, instanceOf(MatchNoDocsQuery.class)); - - final GeoShapeQueryBuilder failingQueryBuilder = new GeoShapeQueryBuilder("unmapped", shape); - failingQueryBuilder.ignoreUnmapped(false); - QueryShardException e = expectThrows(QueryShardException.class, () -> failingQueryBuilder.toQuery(createShardContext())); - assertThat(e.getMessage(), containsString("failed to find geo_shape field [unmapped]")); - } - - public void testSerializationFailsUnlessFetched() throws IOException { - QueryBuilder builder = doCreateTestQueryBuilder(); - QueryBuilder queryBuilder = Rewriteable.rewrite(builder, createShardContext()); - IllegalStateException ise = expectThrows(IllegalStateException.class, () -> queryBuilder.writeTo(new BytesStreamOutput(10))); - assertEquals(ise.getMessage(), "supplier must be null, can't serialize suppliers, missing a rewriteAndFetch?"); - builder = rewriteAndFetch(builder, createShardContext()); - builder.writeTo(new BytesStreamOutput(10)); - } - - @Override - public void testFromXContent() throws IOException { - super.testFromXContent(); - assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); - } - - @Override - public void testUnknownField() throws IOException { - super.testUnknownField(); - assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); - } - - @Override - public void testValidOutput() throws IOException { - super.testValidOutput(); - assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); - } -} diff --git a/server/src/test/java/org/elasticsearch/index/query/IdsQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/IdsQueryBuilderTests.java index 9c9afd70a2fc6..2aed8202dd698 100644 --- a/server/src/test/java/org/elasticsearch/index/query/IdsQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/IdsQueryBuilderTests.java @@ -19,15 +19,19 @@ package org.elasticsearch.index.query; + import org.apache.lucene.search.MatchNoDocsQuery; import org.apache.lucene.search.Query; import org.apache.lucene.search.TermInSetQuery; +import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.common.ParsingException; +import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.mapper.IdFieldMapper; import org.elasticsearch.search.internal.SearchContext; import org.elasticsearch.test.AbstractQueryTestCase; import java.io.IOException; +import java.util.Arrays; import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.Matchers.contains; @@ -37,19 +41,44 @@ public class IdsQueryBuilderTests extends AbstractQueryTestCase @Override protected IdsQueryBuilder doCreateTestQueryBuilder() { - IdsQueryBuilder query = new IdsQueryBuilder(); + final String type; + if (randomBoolean()) { + if (frequently()) { + type = "_doc"; + } else { + type = randomAlphaOfLengthBetween(1, 10); + } + } else if (randomBoolean()) { + type = MetaData.ALL; + } else { + type = null; + } int numberOfIds = randomIntBetween(0, 10); String[] ids = new String[numberOfIds]; for (int i = 0; i < numberOfIds; i++) { ids[i] = randomAlphaOfLengthBetween(1, 10); } - query.addIds(ids); + IdsQueryBuilder query; + if (type != null && randomBoolean()) { + query = new IdsQueryBuilder().types(type); + query.addIds(ids); + } else { + query = new IdsQueryBuilder(); + query.addIds(ids); + } return query; } @Override protected void doAssertLuceneQuery(IdsQueryBuilder queryBuilder, Query query, SearchContext context) throws IOException { - if (queryBuilder.ids().size() == 0 || context.getQueryShardContext().fieldMapper(IdFieldMapper.NAME) == null) { + boolean allTypes = queryBuilder.types().length == 0 || + queryBuilder.types().length == 1 && "_all".equals(queryBuilder.types()[0]); + if (queryBuilder.ids().size() == 0 + // no types + || context.getQueryShardContext().fieldMapper(IdFieldMapper.NAME) == null + // there are types, but disjoint from the query + || (allTypes == false && + Arrays.asList(queryBuilder.types()).indexOf(context.mapperService().documentMapper().type()) == -1)) { assertThat(query, instanceOf(MatchNoDocsQuery.class)); } else { assertThat(query, instanceOf(TermInSetQuery.class)); @@ -57,8 +86,11 @@ protected void doAssertLuceneQuery(IdsQueryBuilder queryBuilder, Query query, Se } public void testIllegalArguments() { + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> new IdsQueryBuilder().types((String[]) null)); + assertEquals("[ids] types cannot be null", e.getMessage()); + IdsQueryBuilder idsQueryBuilder = new IdsQueryBuilder(); - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> idsQueryBuilder.addIds((String[]) null)); + e = expectThrows(IllegalArgumentException.class, () -> idsQueryBuilder.addIds((String[]) null)); assertEquals("[ids] ids cannot be null", e.getMessage()); } @@ -73,14 +105,64 @@ public void testFromJson() throws IOException { String json = "{\n" + " \"ids\" : {\n" + + " \"type\" : [ \"my_type\" ],\n" + " \"values\" : [ \"1\", \"100\", \"4\" ],\n" + " \"boost\" : 1.0\n" + " }\n" + "}"; IdsQueryBuilder parsed = (IdsQueryBuilder) parseQuery(json); + checkGeneratedJson(json, parsed); + assertThat(parsed.ids(), contains("1","100","4")); + assertEquals(json, "my_type", parsed.types()[0]); + + // check that type that is not an array and also ids that are numbers are parsed + json = + "{\n" + + " \"ids\" : {\n" + + " \"type\" : \"my_type\",\n" + + " \"values\" : [ 1, 100, 4 ],\n" + + " \"boost\" : 1.0\n" + + " }\n" + + "}"; + parsed = (IdsQueryBuilder) parseQuery(json); assertThat(parsed.ids(), contains("1","100","4")); - // can't check for {@code checkGeneratedJson(json, parsed)} as - // even if types are null, IdsQueryBuilder uses empty array for decoding them - // which will make {@code checkGeneratedJson(json, parsed)} fail. + assertEquals(json, "my_type", parsed.types()[0]); + + // check with empty type array + json = + "{\n" + + " \"ids\" : {\n" + + " \"type\" : [ ],\n" + + " \"values\" : [ \"1\", \"100\", \"4\" ],\n" + + " \"boost\" : 1.0\n" + + " }\n" + + "}"; + parsed = (IdsQueryBuilder) parseQuery(json); + assertThat(parsed.ids(), contains("1","100","4")); + assertEquals(json, 0, parsed.types().length); + + // check without type + json = + "{\n" + + " \"ids\" : {\n" + + " \"values\" : [ \"1\", \"100\", \"4\" ],\n" + + " \"boost\" : 1.0\n" + + " }\n" + + "}"; + parsed = (IdsQueryBuilder) parseQuery(json); + assertThat(parsed.ids(), contains("1","100","4")); + assertEquals(json, 0, parsed.types().length); + } + + @Override + protected QueryBuilder parseQuery(XContentParser parser) throws IOException { + QueryBuilder query = super.parseQuery(parser); + assertThat(query, instanceOf(IdsQueryBuilder.class)); + + IdsQueryBuilder idsQuery = (IdsQueryBuilder) query; + if (idsQuery.types().length > 0) { + assertWarnings(IdsQueryBuilder.TYPES_DEPRECATION_MESSAGE); + } + return query; } } diff --git a/server/src/test/java/org/elasticsearch/index/query/IdsQueryBuilderTypedTests.java b/server/src/test/java/org/elasticsearch/index/query/IdsQueryBuilderTypedTests.java deleted file mode 100644 index b268e6ee4a9e8..0000000000000 --- a/server/src/test/java/org/elasticsearch/index/query/IdsQueryBuilderTypedTests.java +++ /dev/null @@ -1,192 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.elasticsearch.index.query; - - -import org.apache.lucene.search.MatchNoDocsQuery; -import org.apache.lucene.search.Query; -import org.apache.lucene.search.TermInSetQuery; -import org.elasticsearch.cluster.metadata.MetaData; -import org.elasticsearch.common.ParsingException; -import org.elasticsearch.index.mapper.IdFieldMapper; -import org.elasticsearch.search.internal.SearchContext; -import org.elasticsearch.test.AbstractQueryTestCase; - -import java.io.IOException; -import java.util.Arrays; - -import static org.hamcrest.CoreMatchers.instanceOf; -import static org.hamcrest.Matchers.contains; -import static org.hamcrest.Matchers.containsString; - -public class IdsQueryBuilderTypedTests extends AbstractQueryTestCase { - - @Override - protected IdsQueryBuilder doCreateTestQueryBuilder() { - final String type; - if (randomBoolean()) { - if (frequently()) { - type = "_doc"; - } else { - type = randomAlphaOfLengthBetween(1, 10); - } - } else { - type = MetaData.ALL; - } - int numberOfIds = randomIntBetween(0, 10); - String[] ids = new String[numberOfIds]; - for (int i = 0; i < numberOfIds; i++) { - ids[i] = randomAlphaOfLengthBetween(1, 10); - } - IdsQueryBuilder query = new IdsQueryBuilder().types(type); - query.addIds(ids); - return query; - } - - @Override - protected void doAssertLuceneQuery(IdsQueryBuilder queryBuilder, Query query, SearchContext context) throws IOException { - boolean allTypes = queryBuilder.types().length == 0 || - queryBuilder.types().length == 1 && "_all".equals(queryBuilder.types()[0]); - if (queryBuilder.ids().size() == 0 - // no types - || context.getQueryShardContext().fieldMapper(IdFieldMapper.NAME) == null - // there are types, but disjoint from the query - || (allTypes == false && - Arrays.asList(queryBuilder.types()).indexOf(context.mapperService().documentMapper().type()) == -1)) { - assertThat(query, instanceOf(MatchNoDocsQuery.class)); - } else { - assertThat(query, instanceOf(TermInSetQuery.class)); - } - } - - public void testIllegalArguments() { - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> new IdsQueryBuilder().types((String[]) null)); - assertEquals("[ids] types cannot be null", e.getMessage()); - - IdsQueryBuilder idsQueryBuilder = new IdsQueryBuilder(); - e = expectThrows(IllegalArgumentException.class, () -> idsQueryBuilder.addIds((String[]) null)); - assertEquals("[ids] ids cannot be null", e.getMessage()); - } - - // see #7686. - public void testIdsQueryWithInvalidValues() throws Exception { - String query = "{ \"ids\": { \"values\": [[1]] } }"; - ParsingException e = expectThrows(ParsingException.class, () -> parseQuery(query)); - assertThat(e.getMessage(), containsString("[ids] failed to parse field [values]")); - } - - public void testFromJson() throws IOException { - String json = - "{\n" + - " \"ids\" : {\n" + - " \"type\" : [ \"my_type\" ],\n" + - " \"values\" : [ \"1\", \"100\", \"4\" ],\n" + - " \"boost\" : 1.0\n" + - " }\n" + - "}"; - IdsQueryBuilder parsed = (IdsQueryBuilder) parseQuery(json); - checkGeneratedJson(json, parsed); - assertThat(parsed.ids(), contains("1","100","4")); - assertEquals(json, "my_type", parsed.types()[0]); - - // check that type that is not an array and also ids that are numbers are parsed - json = - "{\n" + - " \"ids\" : {\n" + - " \"type\" : \"my_type\",\n" + - " \"values\" : [ 1, 100, 4 ],\n" + - " \"boost\" : 1.0\n" + - " }\n" + - "}"; - parsed = (IdsQueryBuilder) parseQuery(json); - assertThat(parsed.ids(), contains("1","100","4")); - assertEquals(json, "my_type", parsed.types()[0]); - - // check with empty type array - json = - "{\n" + - " \"ids\" : {\n" + - " \"type\" : [ ],\n" + - " \"values\" : [ \"1\", \"100\", \"4\" ],\n" + - " \"boost\" : 1.0\n" + - " }\n" + - "}"; - parsed = (IdsQueryBuilder) parseQuery(json); - assertThat(parsed.ids(), contains("1","100","4")); - assertEquals(json, 0, parsed.types().length); - - assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); - } - - @Override - public void testToQuery() throws IOException { - super.testToQuery(); - assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); - } - - @Override - public void testEqualsAndHashcode() { - super.testEqualsAndHashcode(); - assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); - } - - @Override - public void testUnknownField() throws IOException { - super.testUnknownField(); - assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); - } - - @Override - public void testUnknownObjectException() throws IOException { - super.testUnknownObjectException(); - assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); - } - - @Override - public void testNegativeBoosts() { - super.testNegativeBoosts(); - assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); - } - @Override - public void testMustRewrite() throws IOException { - super.testMustRewrite(); - assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); - } - @Override - public void testQueryWrappedInArray() { - super.testQueryWrappedInArray(); - assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); - } - @Override - public void testValidOutput() throws IOException { - super.testValidOutput(); - assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); - } - @Override - public void testFromXContent() throws IOException { - super.testFromXContent(); - assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); - } - @Override - public void testSerialization() throws IOException { - super.testSerialization(); - assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); - } -} diff --git a/server/src/test/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilderTests.java index 27dec74173e81..ead5b6b5f5e71 100644 --- a/server/src/test/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilderTests.java @@ -95,11 +95,18 @@ private Item generateRandomItem() { String index = randomBoolean() ? getIndex().getName() : null; // indexed item or artificial document Item item; + if (randomBoolean()) { - item = new Item(index, randomAlphaOfLength(10)); + item = randomBoolean() + ? new Item(index, randomAlphaOfLength(10)) + : new Item(index, randomArtificialDoc()); } else { - item = new Item(index, randomArtificialDoc()); + String type = "doc"; + item = randomBoolean() + ? new Item(index, type, randomAlphaOfLength(10)) + : new Item(index, type, randomArtificialDoc()); } + // if no field is specified MLT uses all mapped fields for this item if (randomBoolean()) { item.fields(randomFrom(randomFields)); @@ -344,9 +351,11 @@ public void testFromJson() throws IOException { " \"fields\" : [ \"title\", \"description\" ],\n" + " \"like\" : [ \"and potentially some more text here as well\", {\n" + " \"_index\" : \"imdb\",\n" + + " \"_type\" : \"movies\",\n" + " \"_id\" : \"1\"\n" + " }, {\n" + " \"_index\" : \"imdb\",\n" + + " \"_type\" : \"movies\",\n" + " \"_id\" : \"2\"\n" + " } ],\n" + " \"max_query_terms\" : 12,\n" + @@ -369,4 +378,16 @@ public void testFromJson() throws IOException { assertEquals(json, 2, parsed.fields().length); assertEquals(json, "and potentially some more text here as well", parsed.likeTexts()[0]); } + + @Override + protected QueryBuilder parseQuery(XContentParser parser) throws IOException { + QueryBuilder query = super.parseQuery(parser); + assertThat(query, instanceOf(MoreLikeThisQueryBuilder.class)); + + MoreLikeThisQueryBuilder mltQuery = (MoreLikeThisQueryBuilder) query; + if (!mltQuery.isTypeless()) { + assertWarnings(MoreLikeThisQueryBuilder.TYPES_DEPRECATION_MESSAGE); + } + return query; + } } diff --git a/server/src/test/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilderTypedTests.java b/server/src/test/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilderTypedTests.java deleted file mode 100644 index 5c5058ae69678..0000000000000 --- a/server/src/test/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilderTypedTests.java +++ /dev/null @@ -1,359 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.elasticsearch.index.query; - -import org.apache.lucene.analysis.core.WhitespaceAnalyzer; -import org.apache.lucene.index.Fields; -import org.apache.lucene.index.memory.MemoryIndex; -import org.apache.lucene.search.BooleanClause; -import org.apache.lucene.search.BooleanQuery; -import org.apache.lucene.search.Query; -import org.elasticsearch.ElasticsearchException; -import org.elasticsearch.action.termvectors.MultiTermVectorsItemResponse; -import org.elasticsearch.action.termvectors.MultiTermVectorsRequest; -import org.elasticsearch.action.termvectors.MultiTermVectorsResponse; -import org.elasticsearch.action.termvectors.TermVectorsRequest; -import org.elasticsearch.action.termvectors.TermVectorsResponse; -import org.elasticsearch.common.Strings; -import org.elasticsearch.common.bytes.BytesReference; -import org.elasticsearch.common.io.stream.BytesStreamOutput; -import org.elasticsearch.common.lucene.search.MoreLikeThisQuery; -import org.elasticsearch.common.xcontent.ToXContent; -import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.common.xcontent.XContentFactory; -import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.common.xcontent.json.JsonXContent; -import org.elasticsearch.index.VersionType; -import org.elasticsearch.index.query.MoreLikeThisQueryBuilder.Item; -import org.elasticsearch.search.internal.SearchContext; -import org.elasticsearch.test.AbstractQueryTestCase; -import org.junit.Before; - -import java.io.IOException; -import java.util.Arrays; -import java.util.Collections; -import java.util.EnumSet; -import java.util.HashMap; -import java.util.Map; -import java.util.Set; -import java.util.stream.Stream; - -import static org.elasticsearch.index.query.QueryBuilders.moreLikeThisQuery; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.greaterThan; -import static org.hamcrest.Matchers.instanceOf; - -public class MoreLikeThisQueryBuilderTypedTests extends AbstractQueryTestCase { - - private static final String[] SHUFFLE_PROTECTED_FIELDS = new String[]{MoreLikeThisQueryBuilder.DOC.getPreferredName()}; - - private static String[] randomFields; - private static Item[] randomLikeItems; - private static Item[] randomUnlikeItems; - - @Before - public void setup() { - randomFields = randomStringFields(); - randomLikeItems = new Item[randomIntBetween(1, 3)]; - for (int i = 0; i < randomLikeItems.length; i++) { - randomLikeItems[i] = generateRandomItem(); - } - randomUnlikeItems = new Item[randomIntBetween(1, 3)]; - for (int i = 0; i < randomUnlikeItems.length; i++) { - randomUnlikeItems[i] = generateRandomItem(); - } - } - - private static String[] randomStringFields() { - String[] mappedStringFields = new String[]{STRING_FIELD_NAME, STRING_FIELD_NAME_2, STRING_ALIAS_FIELD_NAME}; - String[] unmappedStringFields = generateRandomStringArray(2, 5, false, false); - return Stream.concat(Arrays.stream(mappedStringFields), Arrays.stream(unmappedStringFields)).toArray(String[]::new); - } - - private Item generateRandomItem() { - String index = randomBoolean() ? getIndex().getName() : null; - String type = "doc"; - // indexed item or artificial document - Item item; - if (randomBoolean()) { - item = new Item(index, type, randomAlphaOfLength(10)); - } else { - item = new Item(index, type, randomArtificialDoc()); - } - // if no field is specified MLT uses all mapped fields for this item - if (randomBoolean()) { - item.fields(randomFrom(randomFields)); - } - // per field analyzer - if (randomBoolean()) { - item.perFieldAnalyzer(randomPerFieldAnalyzer()); - } - if (randomBoolean()) { - item.routing(randomAlphaOfLength(10)); - } - if (randomBoolean()) { - item.version(randomInt(5)); - } - if (randomBoolean()) { - item.versionType(randomFrom(VersionType.values())); - } - return item; - } - - private XContentBuilder randomArtificialDoc() { - XContentBuilder doc; - try { - doc = XContentFactory.jsonBuilder().startObject(); - for (String field : randomFields) { - doc.field(field, randomAlphaOfLength(10)); - } - doc.endObject(); - } catch (IOException e) { - throw new ElasticsearchException("Unable to generate random artificial doc!"); - } - return doc; - } - - private Map randomPerFieldAnalyzer() { - Map perFieldAnalyzer = new HashMap<>(); - for (String field : randomFields) { - perFieldAnalyzer.put(field, randomAnalyzer()); - } - return perFieldAnalyzer; - } - - @Override - protected MoreLikeThisQueryBuilder doCreateTestQueryBuilder() { - MoreLikeThisQueryBuilder queryBuilder = new MoreLikeThisQueryBuilder(randomFields, null, randomLikeItems); - if (randomBoolean()) { - queryBuilder.unlike(randomUnlikeItems); - } - if (randomBoolean()) { - queryBuilder.maxQueryTerms(randomInt(25)); - } - if (randomBoolean()) { - queryBuilder.minTermFreq(randomInt(5)); - } - if (randomBoolean()) { - queryBuilder.minDocFreq(randomInt(5)); - } - if (randomBoolean()) { - queryBuilder.maxDocFreq(randomInt(100)); - } - if (randomBoolean()) { - queryBuilder.minWordLength(randomInt(5)); - } - if (randomBoolean()) { - queryBuilder.maxWordLength(randomInt(25)); - } - if (randomBoolean()) { - queryBuilder.stopWords(generateRandomStringArray(5, 5, false, false)); - } - if (randomBoolean()) { - queryBuilder.analyzer(randomAnalyzer()); // fix the analyzer? - } - if (randomBoolean()) { - queryBuilder.minimumShouldMatch(randomMinimumShouldMatch()); - } - if (randomBoolean()) { - queryBuilder.boostTerms(randomFloat() * 10); - } - if (randomBoolean()) { - queryBuilder.include(randomBoolean()); - } - if (randomBoolean()) { - queryBuilder.failOnUnsupportedField(randomBoolean()); - } - return queryBuilder; - } - - /** - * we don't want to shuffle the "doc" field internally in {@link #testFromXContent()} because even though the - * documents would be functionally the same, their {@link BytesReference} representation isn't and thats what we - * compare when check for equality of the original and the shuffled builder - */ - @Override - protected String[] shuffleProtectedFields() { - return SHUFFLE_PROTECTED_FIELDS; - } - - @Override - protected Set getObjectsHoldingArbitraryContent() { - //doc contains arbitrary content, anything can be added to it and no exception will be thrown - return Collections.singleton(MoreLikeThisQueryBuilder.DOC.getPreferredName()); - } - - @Override - protected MultiTermVectorsResponse executeMultiTermVectors(MultiTermVectorsRequest mtvRequest) { - try { - MultiTermVectorsItemResponse[] responses = new MultiTermVectorsItemResponse[mtvRequest.size()]; - int i = 0; - for (TermVectorsRequest request : mtvRequest) { - TermVectorsResponse response = new TermVectorsResponse(request.index(), request.type(), request.id()); - response.setExists(true); - Fields generatedFields; - if (request.doc() != null) { - generatedFields = generateFields(randomFields, request.doc().utf8ToString()); - } else { - generatedFields = - generateFields(request.selectedFields().toArray(new String[request.selectedFields().size()]), request.id()); - } - EnumSet flags = EnumSet.of(TermVectorsRequest.Flag.Positions, TermVectorsRequest.Flag.Offsets); - response.setFields(generatedFields, request.selectedFields(), flags, generatedFields); - responses[i++] = new MultiTermVectorsItemResponse(response, null); - } - return new MultiTermVectorsResponse(responses); - } catch (IOException ex) { - throw new ElasticsearchException("boom", ex); - } - } - - /** - * Here we could go overboard and use a pre-generated indexed random document for a given Item, - * but for now we'd prefer to simply return the id as the content of the document and that for - * every field. - */ - private static Fields generateFields(String[] fieldNames, String text) throws IOException { - MemoryIndex index = new MemoryIndex(); - for (String fieldName : fieldNames) { - index.addField(fieldName, text, new WhitespaceAnalyzer()); - } - return index.createSearcher().getIndexReader().getTermVectors(0); - } - - @Override - protected void doAssertLuceneQuery(MoreLikeThisQueryBuilder queryBuilder, Query query, SearchContext context) throws IOException { - if (queryBuilder.likeItems() != null && queryBuilder.likeItems().length > 0) { - assertThat(query, instanceOf(BooleanQuery.class)); - BooleanQuery booleanQuery = (BooleanQuery) query; - for (BooleanClause booleanClause : booleanQuery) { - if (booleanClause.getQuery() instanceof MoreLikeThisQuery) { - MoreLikeThisQuery moreLikeThisQuery = (MoreLikeThisQuery) booleanClause.getQuery(); - assertThat(moreLikeThisQuery.getLikeFields().length, greaterThan(0)); - } - } - } else { - // we rely on integration tests for a deeper check here - assertThat(query, instanceOf(MoreLikeThisQuery.class)); - } - } - - public void testMoreLikeThisBuilder() throws Exception { - Query parsedQuery = - parseQuery(moreLikeThisQuery(new String[]{"name.first", "name.last"}, new String[]{"something"}, null) - .minTermFreq(1).maxQueryTerms(12)).toQuery(createShardContext()); - assertThat(parsedQuery, instanceOf(MoreLikeThisQuery.class)); - MoreLikeThisQuery mltQuery = (MoreLikeThisQuery) parsedQuery; - assertThat(mltQuery.getMoreLikeFields()[0], equalTo("name.first")); - assertThat(mltQuery.getLikeText(), equalTo("something")); - assertThat(mltQuery.getMinTermFrequency(), equalTo(1)); - assertThat(mltQuery.getMaxQueryTerms(), equalTo(12)); - } - - public void testItemSerialization() throws IOException { - Item expectedItem = generateRandomItem(); - BytesStreamOutput output = new BytesStreamOutput(); - expectedItem.writeTo(output); - Item newItem = new Item(output.bytes().streamInput()); - assertEquals(expectedItem, newItem); - } - - public void testItemCopy() throws IOException { - Item expectedItem = generateRandomItem(); - Item newItem = new Item(expectedItem); - assertEquals(expectedItem, newItem); - } - - public void testItemFromXContent() throws IOException { - Item expectedItem = generateRandomItem(); - String json = Strings.toString(expectedItem.toXContent(XContentFactory.jsonBuilder(), ToXContent.EMPTY_PARAMS)); - XContentParser parser = createParser(JsonXContent.jsonXContent, json); - Item newItem = Item.parse(parser, new Item()); - assertEquals(expectedItem, newItem); - - assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); - } - - @Override - protected boolean isCachable(MoreLikeThisQueryBuilder queryBuilder) { - return queryBuilder.likeItems().length == 0; // items are always fetched - } - - public void testFromJson() throws IOException { - String json = - "{\n" + - " \"more_like_this\" : {\n" + - " \"fields\" : [ \"title\", \"description\" ],\n" + - " \"like\" : [ \"and potentially some more text here as well\", {\n" + - " \"_index\" : \"imdb\",\n" + - " \"_type\" : \"movies\",\n" + - " \"_id\" : \"1\"\n" + - " }, {\n" + - " \"_index\" : \"imdb\",\n" + - " \"_type\" : \"movies\",\n" + - " \"_id\" : \"2\"\n" + - " } ],\n" + - " \"max_query_terms\" : 12,\n" + - " \"min_term_freq\" : 1,\n" + - " \"min_doc_freq\" : 5,\n" + - " \"max_doc_freq\" : 2147483647,\n" + - " \"min_word_length\" : 0,\n" + - " \"max_word_length\" : 0,\n" + - " \"minimum_should_match\" : \"30%\",\n" + - " \"boost_terms\" : 0.0,\n" + - " \"include\" : false,\n" + - " \"fail_on_unsupported_field\" : true,\n" + - " \"boost\" : 1.0\n" + - " }\n" + - "}"; - - MoreLikeThisQueryBuilder parsed = (MoreLikeThisQueryBuilder) parseQuery(json); - checkGeneratedJson(json, parsed); - - assertEquals(json, 2, parsed.fields().length); - assertEquals(json, "and potentially some more text here as well", parsed.likeTexts()[0]); - - assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); - } - - @Override - public void testFromXContent() throws IOException { - super.testFromXContent(); - assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); - } - - @Override - public void testUnknownField() throws IOException { - super.testUnknownField(); - assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); - } - - @Override - public void testValidOutput() throws IOException { - super.testValidOutput(); - assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); - } - - @Override - public void testUnknownObjectException() throws IOException { - super.testUnknownObjectException(); - assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); - } -} diff --git a/server/src/test/java/org/elasticsearch/index/query/RandomQueryBuilder.java b/server/src/test/java/org/elasticsearch/index/query/RandomQueryBuilder.java index ecd767b9d657f..2dd4b7c552d9f 100644 --- a/server/src/test/java/org/elasticsearch/index/query/RandomQueryBuilder.java +++ b/server/src/test/java/org/elasticsearch/index/query/RandomQueryBuilder.java @@ -26,6 +26,7 @@ import static org.elasticsearch.test.AbstractBuilderTestCase.STRING_ALIAS_FIELD_NAME; import static org.elasticsearch.test.AbstractBuilderTestCase.STRING_FIELD_NAME; +import static org.elasticsearch.test.ESTestCase.randomAlphaOfLength; import static org.elasticsearch.test.ESTestCase.randomFrom; /** @@ -47,7 +48,7 @@ public static QueryBuilder createQuery(Random r) { case 1: return new TermQueryBuilderTests().createTestQueryBuilder(); case 2: - return new IdsQueryBuilderTests().createTestQueryBuilder(); + return new ExistsQueryBuilder(randomAlphaOfLength(5)); case 3: return createMultiTermQuery(r); default: diff --git a/server/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java index dd4e378621270..5e66cd6f9c01c 100644 --- a/server/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java @@ -35,10 +35,12 @@ import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.get.GetResult; import org.elasticsearch.indices.TermsLookup; import org.elasticsearch.search.internal.SearchContext; import org.elasticsearch.test.AbstractQueryTestCase; +import org.hamcrest.CoreMatchers; import org.junit.Before; import java.io.IOException; @@ -97,11 +99,11 @@ protected TermsQueryBuilder doCreateTestQueryBuilder() { } private TermsLookup randomTermsLookup() { - return new TermsLookup( - randomAlphaOfLength(10), - randomAlphaOfLength(10), - termsPath - ).routing(randomBoolean() ? randomAlphaOfLength(10) : null); + TermsLookup lookup = randomBoolean() + ? new TermsLookup(randomAlphaOfLength(10), randomAlphaOfLength(10), termsPath) + : new TermsLookup(randomAlphaOfLength(10), randomAlphaOfLength(10), randomAlphaOfLength(10), termsPath); + lookup.routing(randomBoolean() ? randomAlphaOfLength(10) : null); + return lookup; } @Override @@ -181,6 +183,7 @@ public void testBothValuesAndLookupSet() throws IOException { " ],\n" + " \"field_lookup\": {\n" + " \"index\": \"pills\",\n" + + " \"type\": \"red\",\n" + " \"id\": \"3\",\n" + " \"path\": \"white rabbit\"\n" + " }\n" + @@ -318,5 +321,16 @@ public void testTypeField() throws IOException { builder.doToQuery(createShardContext()); assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); } -} + @Override + protected QueryBuilder parseQuery(XContentParser parser) throws IOException { + QueryBuilder query = super.parseQuery(parser); + assertThat(query, CoreMatchers.instanceOf(TermsQueryBuilder.class)); + + TermsQueryBuilder termsQuery = (TermsQueryBuilder) query; + if (termsQuery.termsLookup() != null && termsQuery.termsLookup().type() != null) { + assertWarnings(TermsQueryBuilder.TYPES_DEPRECATION_MESSAGE); + } + return query; + } +} diff --git a/server/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTypedTests.java b/server/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTypedTests.java deleted file mode 100644 index 2bff1f106403a..0000000000000 --- a/server/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTypedTests.java +++ /dev/null @@ -1,148 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.elasticsearch.index.query; - -import org.apache.lucene.search.BooleanQuery; -import org.apache.lucene.search.ConstantScoreQuery; -import org.apache.lucene.search.MatchNoDocsQuery; -import org.apache.lucene.search.PointInSetQuery; -import org.apache.lucene.search.Query; -import org.apache.lucene.search.TermInSetQuery; -import org.apache.lucene.util.BytesRef; -import org.elasticsearch.ElasticsearchException; -import org.elasticsearch.action.get.GetRequest; -import org.elasticsearch.action.get.GetResponse; -import org.elasticsearch.common.Strings; -import org.elasticsearch.common.bytes.BytesArray; -import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.common.xcontent.XContentFactory; -import org.elasticsearch.index.get.GetResult; -import org.elasticsearch.indices.TermsLookup; -import org.elasticsearch.search.internal.SearchContext; -import org.elasticsearch.test.AbstractQueryTestCase; -import org.junit.Before; - -import java.io.IOException; -import java.util.ArrayList; -import java.util.List; -import java.util.Objects; -import java.util.stream.Collectors; - -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.either; -import static org.hamcrest.Matchers.instanceOf; - -public class TermsQueryBuilderTypedTests extends AbstractQueryTestCase { - private List randomTerms; - private String termsPath; - - @Before - public void randomTerms() { - List randomTerms = new ArrayList<>(); - String[] strings = generateRandomStringArray(10, 10, false, true); - for (String string : strings) { - randomTerms.add(string); - if (rarely()) { - randomTerms.add(null); - } - } - this.randomTerms = randomTerms; - this.termsPath = randomAlphaOfLength(10).replace('.', '_'); - } - - @Override - protected TermsQueryBuilder doCreateTestQueryBuilder() { - TermsQueryBuilder query = new TermsQueryBuilder( - randomBoolean() ? randomAlphaOfLengthBetween(1,10) : STRING_FIELD_NAME, randomTermsLookup()); - return query; - } - - private TermsLookup randomTermsLookup() { - return new TermsLookup( - randomAlphaOfLength(10), - randomAlphaOfLength(10), - randomAlphaOfLength(10), - termsPath - ).routing(randomBoolean() ? randomAlphaOfLength(10) : null); - } - - @Override - protected void doAssertLuceneQuery(TermsQueryBuilder queryBuilder, Query query, SearchContext context) throws IOException { - if (queryBuilder.termsLookup() != null && randomTerms.size() == 0){ - assertThat(query, instanceOf(MatchNoDocsQuery.class)); - MatchNoDocsQuery matchNoDocsQuery = (MatchNoDocsQuery) query; - assertThat(matchNoDocsQuery.toString(), containsString("No terms supplied for \"terms\" query.")); - } else { - assertThat(query, either(instanceOf(TermInSetQuery.class)) - .or(instanceOf(PointInSetQuery.class)) - .or(instanceOf(ConstantScoreQuery.class))); - if (query instanceof ConstantScoreQuery) { - assertThat(((ConstantScoreQuery) query).getQuery(), instanceOf(BooleanQuery.class)); - } - String fieldName = expectedFieldName(queryBuilder.fieldName()); - TermInSetQuery expected = new TermInSetQuery(fieldName, - randomTerms.stream().filter(Objects::nonNull).map(Object::toString).map(BytesRef::new).collect(Collectors.toList())); - assertEquals(expected, query); - } - } - - @Override - public GetResponse executeGet(GetRequest getRequest) { - String json; - try { - XContentBuilder builder = XContentFactory.jsonBuilder().prettyPrint(); - builder.startObject(); - builder.array(termsPath, randomTerms.toArray(new Object[randomTerms.size()])); - builder.endObject(); - json = Strings.toString(builder); - } catch (IOException ex) { - throw new ElasticsearchException("boom", ex); - } - return new GetResponse(new GetResult(getRequest.index(), getRequest.type(), getRequest.id(), 0, 1, 0, true, - new BytesArray(json), null)); - } - - @Override - public void testMustRewrite() throws IOException { - TermsQueryBuilder termsQueryBuilder = new TermsQueryBuilder(STRING_FIELD_NAME, randomTermsLookup()); - UnsupportedOperationException e = expectThrows(UnsupportedOperationException.class, - () -> termsQueryBuilder.toQuery(createShardContext())); - assertEquals("query must be rewritten first", e.getMessage()); - } - - @Override - public void testFromXContent() throws IOException { - super.testFromXContent(); - assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); - } - - @Override - public void testUnknownField() throws IOException { - super.testUnknownField(); - assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); - } - - @Override - public void testValidOutput() throws IOException { - super.testValidOutput(); - assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE); - } -} - diff --git a/server/src/test/java/org/elasticsearch/search/geo/GeoShapeQueryTests.java b/server/src/test/java/org/elasticsearch/search/geo/GeoShapeQueryTests.java index a64f98df5a6eb..35beb10934e3d 100644 --- a/server/src/test/java/org/elasticsearch/search/geo/GeoShapeQueryTests.java +++ b/server/src/test/java/org/elasticsearch/search/geo/GeoShapeQueryTests.java @@ -195,7 +195,44 @@ public void testIndexedShapeReference() throws Exception { .endObject() .endObject()).setRefreshPolicy(IMMEDIATE).get(); - SearchResponse searchResponse = client().prepareSearch("test").setTypes("type1") + SearchResponse searchResponse = client().prepareSearch("test") + .setQuery(geoIntersectionQuery("location", "Big_Rectangle")) + .get(); + + assertSearchResponse(searchResponse); + assertThat(searchResponse.getHits().getTotalHits().value, equalTo(1L)); + assertThat(searchResponse.getHits().getHits().length, equalTo(1)); + assertThat(searchResponse.getHits().getAt(0).getId(), equalTo("1")); + + searchResponse = client().prepareSearch("test") + .setQuery(geoShapeQuery("location", "Big_Rectangle")) + .get(); + + assertSearchResponse(searchResponse); + assertThat(searchResponse.getHits().getTotalHits().value, equalTo(1L)); + assertThat(searchResponse.getHits().getHits().length, equalTo(1)); + assertThat(searchResponse.getHits().getAt(0).getId(), equalTo("1")); + } + + public void testIndexedShapeReferenceWithTypes() throws Exception { + String mapping = Strings.toString(createMapping()); + client().admin().indices().prepareCreate("test").addMapping("type1", mapping, XContentType.JSON).get(); + createIndex("shapes"); + ensureGreen(); + + EnvelopeBuilder shape = new EnvelopeBuilder(new Coordinate(-45, 45), new Coordinate(45, -45)); + + client().prepareIndex("shapes", "shape_type", "Big_Rectangle").setSource(jsonBuilder().startObject() + .field("shape", shape).endObject()).setRefreshPolicy(IMMEDIATE).get(); + client().prepareIndex("test", "type1", "1").setSource(jsonBuilder().startObject() + .field("name", "Document 1") + .startObject("location") + .field("type", "point") + .startArray("coordinates").value(-30).value(-30).endArray() + .endObject() + .endObject()).setRefreshPolicy(IMMEDIATE).get(); + + SearchResponse searchResponse = client().prepareSearch("test") .setQuery(geoIntersectionQuery("location", "Big_Rectangle", "shape_type")) .get(); @@ -225,8 +262,8 @@ public void testIndexedShapeReferenceSourceDisabled() throws Exception { client().prepareIndex("shapes", "shape_type", "Big_Rectangle").setSource(jsonBuilder().startObject() .field("shape", shape).endObject()).setRefreshPolicy(IMMEDIATE).get(); - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> client().prepareSearch("test").setTypes("type1") - .setQuery(geoIntersectionQuery("location", "Big_Rectangle", "shape_type")).get()); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> client().prepareSearch("test") + .setQuery(geoIntersectionQuery("location", "Big_Rectangle")).get()); assertThat(e.getMessage(), containsString("source disabled")); } @@ -273,28 +310,28 @@ public void testShapeFetchingPath() throws Exception { .endArray().endArray() .endObject().endObject()).setRefreshPolicy(IMMEDIATE).get(); - GeoShapeQueryBuilder filter = QueryBuilders.geoShapeQuery("location", "1", "type").relation(ShapeRelation.INTERSECTS) + GeoShapeQueryBuilder filter = QueryBuilders.geoShapeQuery("location", "1").relation(ShapeRelation.INTERSECTS) .indexedShapeIndex("shapes") .indexedShapePath("location"); SearchResponse result = client().prepareSearch("test").setQuery(QueryBuilders.matchAllQuery()) .setPostFilter(filter).get(); assertSearchResponse(result); assertHitCount(result, 1); - filter = QueryBuilders.geoShapeQuery("location", "1", "type").relation(ShapeRelation.INTERSECTS) + filter = QueryBuilders.geoShapeQuery("location", "1").relation(ShapeRelation.INTERSECTS) .indexedShapeIndex("shapes") .indexedShapePath("1.location"); result = client().prepareSearch("test").setQuery(QueryBuilders.matchAllQuery()) .setPostFilter(filter).get(); assertSearchResponse(result); assertHitCount(result, 1); - filter = QueryBuilders.geoShapeQuery("location", "1", "type").relation(ShapeRelation.INTERSECTS) + filter = QueryBuilders.geoShapeQuery("location", "1").relation(ShapeRelation.INTERSECTS) .indexedShapeIndex("shapes") .indexedShapePath("1.2.location"); result = client().prepareSearch("test").setQuery(QueryBuilders.matchAllQuery()) .setPostFilter(filter).get(); assertSearchResponse(result); assertHitCount(result, 1); - filter = QueryBuilders.geoShapeQuery("location", "1", "type").relation(ShapeRelation.INTERSECTS) + filter = QueryBuilders.geoShapeQuery("location", "1").relation(ShapeRelation.INTERSECTS) .indexedShapeIndex("shapes") .indexedShapePath("1.2.3.location"); result = client().prepareSearch("test").setQuery(QueryBuilders.matchAllQuery()) @@ -303,25 +340,25 @@ public void testShapeFetchingPath() throws Exception { assertHitCount(result, 1); // now test the query variant - GeoShapeQueryBuilder query = QueryBuilders.geoShapeQuery("location", "1", "type") + GeoShapeQueryBuilder query = QueryBuilders.geoShapeQuery("location", "1") .indexedShapeIndex("shapes") .indexedShapePath("location"); result = client().prepareSearch("test").setQuery(query).get(); assertSearchResponse(result); assertHitCount(result, 1); - query = QueryBuilders.geoShapeQuery("location", "1", "type") + query = QueryBuilders.geoShapeQuery("location", "1") .indexedShapeIndex("shapes") .indexedShapePath("1.location"); result = client().prepareSearch("test").setQuery(query).get(); assertSearchResponse(result); assertHitCount(result, 1); - query = QueryBuilders.geoShapeQuery("location", "1", "type") + query = QueryBuilders.geoShapeQuery("location", "1") .indexedShapeIndex("shapes") .indexedShapePath("1.2.location"); result = client().prepareSearch("test").setQuery(query).get(); assertSearchResponse(result); assertHitCount(result, 1); - query = QueryBuilders.geoShapeQuery("location", "1", "type") + query = QueryBuilders.geoShapeQuery("location", "1") .indexedShapeIndex("shapes") .indexedShapePath("1.2.3.location"); result = client().prepareSearch("test").setQuery(query).get(); @@ -356,7 +393,7 @@ public void testQueryRandomGeoCollection() throws Exception { GeoShapeQueryBuilder geoShapeQueryBuilder = QueryBuilders.geoShapeQuery("location", filterShape); geoShapeQueryBuilder.relation(ShapeRelation.INTERSECTS); - SearchResponse result = client().prepareSearch("test").setTypes("type").setQuery(geoShapeQueryBuilder).get(); + SearchResponse result = client().prepareSearch("test").setQuery(geoShapeQueryBuilder).get(); assertSearchResponse(result); assertHitCount(result, 1); } @@ -405,7 +442,7 @@ public void testRandomGeoCollectionQuery() throws Exception { GeoShapeQueryBuilder geoShapeQueryBuilder = QueryBuilders.geoShapeQuery("location", queryCollection); geoShapeQueryBuilder.relation(ShapeRelation.INTERSECTS); - SearchResponse result = client().prepareSearch("test").setTypes("type").setQuery(geoShapeQueryBuilder).get(); + SearchResponse result = client().prepareSearch("test").setQuery(geoShapeQueryBuilder).get(); assertSearchResponse(result); assertTrue(result.getHits().getTotalHits().value > 0); } @@ -429,7 +466,7 @@ public void testPointQuery() throws Exception { GeoShapeQueryBuilder geoShapeQueryBuilder = QueryBuilders.geoShapeQuery("location", pb); geoShapeQueryBuilder.relation(ShapeRelation.INTERSECTS); - SearchResponse result = client().prepareSearch("test").setTypes("type").setQuery(geoShapeQueryBuilder).get(); + SearchResponse result = client().prepareSearch("test").setQuery(geoShapeQueryBuilder).get(); assertSearchResponse(result); assertHitCount(result, 1); } @@ -454,7 +491,7 @@ public void testContainsShapeQuery() throws Exception { ShapeBuilder filterShape = (gcb.getShapeAt(randomIntBetween(0, gcb.numShapes() - 1))); GeoShapeQueryBuilder filter = QueryBuilders.geoShapeQuery("location", filterShape) .relation(ShapeRelation.CONTAINS); - SearchResponse response = client().prepareSearch("test").setTypes("type").setQuery(QueryBuilders.matchAllQuery()) + SearchResponse response = client().prepareSearch("test").setQuery(QueryBuilders.matchAllQuery()) .setPostFilter(filter).get(); assertSearchResponse(response); @@ -478,7 +515,7 @@ public void testExistsQuery() throws Exception { client().prepareIndex("test", "type", "1").setSource(docSource).setRefreshPolicy(IMMEDIATE).get(); ExistsQueryBuilder eqb = QueryBuilders.existsQuery("location"); - SearchResponse result = client().prepareSearch("test").setTypes("type").setQuery(eqb).get(); + SearchResponse result = client().prepareSearch("test").setQuery(eqb).get(); assertSearchResponse(result); assertHitCount(result, 1); } @@ -520,7 +557,7 @@ public void testShapeFilterWithDefinedGeoCollection() throws Exception { new PolygonBuilder(new CoordinatesBuilder().coordinate(99.0, -1.0).coordinate(99.0, 3.0) .coordinate(103.0, 3.0).coordinate(103.0, -1.0) .coordinate(99.0, -1.0)))).relation(ShapeRelation.INTERSECTS); - SearchResponse result = client().prepareSearch("test").setTypes("type").setQuery(QueryBuilders.matchAllQuery()) + SearchResponse result = client().prepareSearch("test").setQuery(QueryBuilders.matchAllQuery()) .setPostFilter(filter).get(); assertSearchResponse(result); assertHitCount(result, 1); @@ -530,7 +567,7 @@ public void testShapeFilterWithDefinedGeoCollection() throws Exception { new PolygonBuilder(new CoordinatesBuilder().coordinate(199.0, -11.0).coordinate(199.0, 13.0) .coordinate(193.0, 13.0).coordinate(193.0, -11.0) .coordinate(199.0, -11.0)))).relation(ShapeRelation.INTERSECTS); - result = client().prepareSearch("test").setTypes("type").setQuery(QueryBuilders.matchAllQuery()) + result = client().prepareSearch("test").setQuery(QueryBuilders.matchAllQuery()) .setPostFilter(filter).get(); assertSearchResponse(result); assertHitCount(result, 0); @@ -541,13 +578,13 @@ public void testShapeFilterWithDefinedGeoCollection() throws Exception { new PolygonBuilder(new CoordinatesBuilder().coordinate(199.0, -11.0).coordinate(199.0, 13.0) .coordinate(193.0, 13.0).coordinate(193.0, -11.0) .coordinate(199.0, -11.0)))).relation(ShapeRelation.INTERSECTS); - result = client().prepareSearch("test").setTypes("type").setQuery(QueryBuilders.matchAllQuery()) + result = client().prepareSearch("test").setQuery(QueryBuilders.matchAllQuery()) .setPostFilter(filter).get(); assertSearchResponse(result); assertHitCount(result, 1); // no shape filter = QueryBuilders.geoShapeQuery("location", new GeometryCollectionBuilder()); - result = client().prepareSearch("test").setTypes("type").setQuery(QueryBuilders.matchAllQuery()) + result = client().prepareSearch("test").setQuery(QueryBuilders.matchAllQuery()) .setPostFilter(filter).get(); assertSearchResponse(result); assertHitCount(result, 0); diff --git a/server/src/test/java/org/elasticsearch/search/morelikethis/MoreLikeThisIT.java b/server/src/test/java/org/elasticsearch/search/morelikethis/MoreLikeThisIT.java index 62684f811643d..36eabf8d6ff68 100644 --- a/server/src/test/java/org/elasticsearch/search/morelikethis/MoreLikeThisIT.java +++ b/server/src/test/java/org/elasticsearch/search/morelikethis/MoreLikeThisIT.java @@ -89,10 +89,34 @@ public void testSimpleMoreLikeThis() throws Exception { logger.info("Running moreLikeThis"); SearchResponse response = client().prepareSearch().setQuery( - new MoreLikeThisQueryBuilder(null, new Item[] {new Item("test", "type1", "1")}).minTermFreq(1).minDocFreq(1)).get(); + new MoreLikeThisQueryBuilder(null, new Item[] {new Item("test", "1")}).minTermFreq(1).minDocFreq(1)).get(); assertHitCount(response, 1L); } + public void testSimpleMoreLikeThisWithTypes() throws Exception { + logger.info("Creating index test"); + assertAcked(prepareCreate("test").addMapping("type1", + jsonBuilder().startObject().startObject("type1").startObject("properties") + .startObject("text").field("type", "text").endObject() + .endObject().endObject().endObject())); + + logger.info("Running Cluster Health"); + assertThat(ensureGreen(), equalTo(ClusterHealthStatus.GREEN)); + + logger.info("Indexing..."); + client().index(indexRequest("test").type("type1").id("1").source(jsonBuilder().startObject().field("text", "lucene").endObject())) + .actionGet(); + client().index(indexRequest("test").type("type1").id("2") + .source(jsonBuilder().startObject().field("text", "lucene release").endObject())).actionGet(); + client().admin().indices().refresh(refreshRequest()).actionGet(); + + logger.info("Running moreLikeThis"); + SearchResponse response = client().prepareSearch().setQuery( + new MoreLikeThisQueryBuilder(null, new Item[] {new Item("test", "type1", "1")}).minTermFreq(1).minDocFreq(1)).get(); + assertHitCount(response, 1L); + } + + //Issue #30148 public void testMoreLikeThisForZeroTokensInOneOfTheAnalyzedFields() throws Exception { CreateIndexRequestBuilder createIndexRequestBuilder = prepareCreate("test") @@ -116,7 +140,7 @@ public void testMoreLikeThisForZeroTokensInOneOfTheAnalyzedFields() throws Excep client().admin().indices().refresh(refreshRequest()).actionGet(); SearchResponse searchResponse = client().prepareSearch().setQuery( - moreLikeThisQuery(new String[]{"myField", "empty"}, null, new Item[]{new Item("test", "type", "1")}) + moreLikeThisQuery(new String[]{"myField", "empty"}, null, new Item[]{new Item("test", "1")}) .minTermFreq(1).minDocFreq(1) ).get(); @@ -142,7 +166,7 @@ public void testSimpleMoreLikeOnLongField() throws Exception { logger.info("Running moreLikeThis"); SearchResponse response = client().prepareSearch().setQuery( - new MoreLikeThisQueryBuilder(null, new Item[] {new Item("test", "type1", "1")}).minTermFreq(1).minDocFreq(1)).get(); + new MoreLikeThisQueryBuilder(null, new Item[] {new Item("test", "1")}).minTermFreq(1).minDocFreq(1)).get(); assertHitCount(response, 0L); } @@ -173,24 +197,24 @@ public void testMoreLikeThisWithAliases() throws Exception { logger.info("Running moreLikeThis on index"); SearchResponse response = client().prepareSearch().setQuery( - new MoreLikeThisQueryBuilder(null, new Item[] {new Item("test", "type1", "1")}).minTermFreq(1).minDocFreq(1)).get(); + new MoreLikeThisQueryBuilder(null, new Item[] {new Item("test", "1")}).minTermFreq(1).minDocFreq(1)).get(); assertHitCount(response, 2L); logger.info("Running moreLikeThis on beta shard"); response = client().prepareSearch("beta").setQuery( - new MoreLikeThisQueryBuilder(null, new Item[] {new Item("test", "type1", "1")}).minTermFreq(1).minDocFreq(1)).get(); + new MoreLikeThisQueryBuilder(null, new Item[] {new Item("test", "1")}).minTermFreq(1).minDocFreq(1)).get(); assertHitCount(response, 1L); assertThat(response.getHits().getAt(0).getId(), equalTo("3")); logger.info("Running moreLikeThis on release shard"); response = client().prepareSearch("release").setQuery( - new MoreLikeThisQueryBuilder(null, new Item[] {new Item("test", "type1", "1")}).minTermFreq(1).minDocFreq(1)).get(); + new MoreLikeThisQueryBuilder(null, new Item[] {new Item("test", "1")}).minTermFreq(1).minDocFreq(1)).get(); assertHitCount(response, 1L); assertThat(response.getHits().getAt(0).getId(), equalTo("2")); logger.info("Running moreLikeThis on alias with node client"); response = internalCluster().coordOnlyNodeClient().prepareSearch("beta").setQuery( - new MoreLikeThisQueryBuilder(null, new Item[] {new Item("test", "type1", "1")}).minTermFreq(1).minDocFreq(1)).get(); + new MoreLikeThisQueryBuilder(null, new Item[] {new Item("test", "1")}).minTermFreq(1).minDocFreq(1)).get(); assertHitCount(response, 1L); assertThat(response.getHits().getAt(0).getId(), equalTo("3")); } @@ -311,13 +335,13 @@ public void testNumericField() throws Exception { // Implicit list of fields -> ignore numeric fields SearchResponse searchResponse = client().prepareSearch().setQuery( - new MoreLikeThisQueryBuilder(null, new Item[] {new Item("test", "type", "1")}).minTermFreq(1).minDocFreq(1)).get(); + new MoreLikeThisQueryBuilder(null, new Item[] {new Item("test", "1")}).minTermFreq(1).minDocFreq(1)).get(); assertHitCount(searchResponse, 1L); // Explicit list of fields including numeric fields -> fail assertThrows(client().prepareSearch().setQuery( new MoreLikeThisQueryBuilder(new String[] {"string_value", "int_value"}, null, - new Item[] {new Item("test", "type", "1")}).minTermFreq(1).minDocFreq(1)), SearchPhaseExecutionException.class); + new Item[] {new Item("test", "1")}).minTermFreq(1).minDocFreq(1)), SearchPhaseExecutionException.class); // mlt query with no field -> No results (because _all is not enabled) searchResponse = client().prepareSearch().setQuery(moreLikeThisQuery(new String[] {"index"}).minTermFreq(1).minDocFreq(1)) @@ -417,7 +441,7 @@ public void testSimpleMoreLikeInclude() throws Exception { logger.info("Running More Like This with include true"); SearchResponse response = client().prepareSearch().setQuery( - new MoreLikeThisQueryBuilder(null, new Item[] {new Item("test", "type1", "1")}).minTermFreq(1).minDocFreq(1).include(true) + new MoreLikeThisQueryBuilder(null, new Item[] {new Item("test", "1")}).minTermFreq(1).minDocFreq(1).include(true) .minimumShouldMatch("0%")).get(); assertOrderedSearchHits(response, "1", "2"); @@ -428,7 +452,7 @@ public void testSimpleMoreLikeInclude() throws Exception { logger.info("Running More Like This with include false"); response = client().prepareSearch().setQuery( - new MoreLikeThisQueryBuilder(null, new Item[] {new Item("test", "type1", "1")}).minTermFreq(1).minDocFreq(1) + new MoreLikeThisQueryBuilder(null, new Item[] {new Item("test", "1")}).minTermFreq(1).minDocFreq(1) .minimumShouldMatch("0%")).get(); assertSearchHits(response, "2"); } @@ -673,7 +697,7 @@ public void testSelectFields() throws IOException, ExecutionException, Interrupt .field("text1", "elasticsearch") .endObject())); - MoreLikeThisQueryBuilder mltQuery = moreLikeThisQuery(new Item[] {new Item("test", "type1", "1")}) + MoreLikeThisQueryBuilder mltQuery = moreLikeThisQuery(new Item[] {new Item("test", "1")}) .minTermFreq(0) .minDocFreq(0) .include(true) @@ -683,7 +707,7 @@ public void testSelectFields() throws IOException, ExecutionException, Interrupt assertSearchResponse(response); assertHitCount(response, 2); - mltQuery = moreLikeThisQuery(new String[] {"text"}, null, new Item[] {new Item("test", "type1", "1")}) + mltQuery = moreLikeThisQuery(new String[] {"text"}, null, new Item[] {new Item("test", "1")}) .minTermFreq(0) .minDocFreq(0) .include(true) @@ -724,7 +748,7 @@ public void testWithMissingRouting() throws IOException { logger.info("Running moreLikeThis with one item without routing attribute"); SearchPhaseExecutionException exception = expectThrows(SearchPhaseExecutionException.class, () -> client().prepareSearch().setQuery(new MoreLikeThisQueryBuilder(null, new Item[]{ - new Item("test", "type1", "1") + new Item("test", "1") }).minTermFreq(1).minDocFreq(1)).get()); Throwable cause = exception.getCause(); @@ -736,7 +760,7 @@ public void testWithMissingRouting() throws IOException { logger.info("Running moreLikeThis with one item with routing attribute and two items without routing attribute"); SearchPhaseExecutionException exception = expectThrows(SearchPhaseExecutionException.class, () -> client().prepareSearch().setQuery(new MoreLikeThisQueryBuilder(null, new Item[]{ - new Item("test", "type1", "1").routing("1"), + new Item("test", "1").routing("1"), new Item("test", "type1", "2"), new Item("test", "type1", "3") }).minTermFreq(1).minDocFreq(1)).get()); diff --git a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java index 01ff8e395a316..a16f55e04d74a 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java @@ -163,7 +163,7 @@ public void testUnknownField() throws IOException { * parse exception. Some specific objects do not cause any exception as they can hold arbitrary content; they can be * declared by overriding {@link #getObjectsHoldingArbitraryContent()}. */ - public void testUnknownObjectException() throws IOException { + public final void testUnknownObjectException() throws IOException { Set candidates = new HashSet<>(); // Adds the valid query to the list of queries to modify and test candidates.add(createTestQueryBuilder().toString()); @@ -321,7 +321,7 @@ protected Set getObjectsHoldingArbitraryContent() { * Test that wraps the randomly generated query into an array as follows: { "query_name" : [{}]} * This causes unexpected situations in parser code that may not be handled properly. */ - public void testQueryWrappedInArray() { + public final void testQueryWrappedInArray() { QB queryBuilder = createTestQueryBuilder(); String queryName = queryBuilder.getName(); String validQuery = queryBuilder.toString(); @@ -379,7 +379,7 @@ protected void assertParsedQuery(String queryAsString, QueryBuilder expectedQuer /** * Parses the query provided as bytes argument and compares it with the expected result provided as argument as a {@link QueryBuilder} */ - private static void assertParsedQuery(XContentParser parser, QueryBuilder expectedQuery) throws IOException { + private void assertParsedQuery(XContentParser parser, QueryBuilder expectedQuery) throws IOException { QueryBuilder newQuery = parseQuery(parser); assertNotSame(newQuery, expectedQuery); assertEquals(expectedQuery, newQuery); @@ -396,7 +396,7 @@ protected QueryBuilder parseQuery(String queryAsString) throws IOException { return parseQuery(parser); } - protected static QueryBuilder parseQuery(XContentParser parser) throws IOException { + protected QueryBuilder parseQuery(XContentParser parser) throws IOException { QueryBuilder parseInnerQueryBuilder = parseInnerQueryBuilder(parser); assertNull(parser.nextToken()); return parseInnerQueryBuilder; From 986930ed5c023e84451c6a4735b2e2cb35d3cd9d Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Mon, 7 Jan 2019 10:52:40 -0800 Subject: [PATCH 03/14] Add Javadoc to deprecated methods to suggest alternatives. --- .../elasticsearch/index/query/GeoShapeQueryBuilder.java | 2 ++ .../org/elasticsearch/index/query/IdsQueryBuilder.java | 4 ++++ .../index/query/MoreLikeThisQueryBuilder.java | 4 ++++ .../main/java/org/elasticsearch/indices/TermsLookup.java | 7 +++++++ 4 files changed, 17 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java index e9837668a09aa..76bd81d6f9ef3 100644 --- a/server/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java @@ -258,6 +258,8 @@ public String 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() { diff --git a/server/src/main/java/org/elasticsearch/index/query/IdsQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/IdsQueryBuilder.java index c42ca72163b8e..358a2fccff108 100644 --- a/server/src/main/java/org/elasticsearch/index/query/IdsQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/IdsQueryBuilder.java @@ -88,6 +88,8 @@ protected void doWriteTo(StreamOutput out) throws IOException { /** * Add types to query + * + * @deprecated Types are in the process of being removed, prefer to filter on a field instead. */ @Deprecated public IdsQueryBuilder types(String... types) { @@ -100,6 +102,8 @@ public IdsQueryBuilder types(String... types) { /** * Returns the types used in this query + * + * @deprecated Types are in the process of being removed, prefer to filter on a field instead. */ @Deprecated public String[] types() { diff --git a/server/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java index aada4e982f3d4..be551cf4c1457 100644 --- a/server/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java @@ -221,6 +221,8 @@ public Item(@Nullable String index, XContentBuilder doc) { * @param index the index where the document is located * @param type the type of the document * @param id and its id + * + * @deprecated Types are in the process of being removed, use {@link Item(String, String)} instead. */ @Deprecated public Item(@Nullable String index, @Nullable String type, String id) { @@ -238,6 +240,8 @@ public Item(@Nullable String index, @Nullable String type, String id) { * @param index the index to be used for parsing the doc * @param type the type to be used for parsing the doc * @param doc the document specification + * + * @deprecated Types are in the process of being removed, use {@link Item(String, XContentBuilder)} instead. */ @Deprecated public Item(@Nullable String index, @Nullable String type, XContentBuilder doc) { diff --git a/server/src/main/java/org/elasticsearch/indices/TermsLookup.java b/server/src/main/java/org/elasticsearch/indices/TermsLookup.java index 195e0401b2f64..31a1afdcf6d56 100644 --- a/server/src/main/java/org/elasticsearch/indices/TermsLookup.java +++ b/server/src/main/java/org/elasticsearch/indices/TermsLookup.java @@ -43,10 +43,14 @@ public class TermsLookup implements Writeable, ToXContentFragment { private final String path; private String routing; + public TermsLookup(String index, String id, String path) { this(index, null, id, path); } + /** + * @deprecated Types are in the process of being removed, use {@link TermsLookup(String, String, String)} instead. + */ @Deprecated public TermsLookup(String index, String type, String id, String path) { if (id == null) { @@ -107,6 +111,9 @@ public String index() { return index; } + /** + * @deprecated Types are in the process of being removed. + */ @Deprecated public String type() { return type; From 96370d78695021b46c48a3a827040df89c863f08 Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Mon, 7 Jan 2019 11:03:33 -0800 Subject: [PATCH 04/14] Switch back to using an ids query in RandomQueryBuilder. --- .../org/elasticsearch/index/query/RandomQueryBuilder.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/index/query/RandomQueryBuilder.java b/server/src/test/java/org/elasticsearch/index/query/RandomQueryBuilder.java index 2dd4b7c552d9f..8eb9ee1af0c0b 100644 --- a/server/src/test/java/org/elasticsearch/index/query/RandomQueryBuilder.java +++ b/server/src/test/java/org/elasticsearch/index/query/RandomQueryBuilder.java @@ -26,7 +26,6 @@ import static org.elasticsearch.test.AbstractBuilderTestCase.STRING_ALIAS_FIELD_NAME; import static org.elasticsearch.test.AbstractBuilderTestCase.STRING_FIELD_NAME; -import static org.elasticsearch.test.ESTestCase.randomAlphaOfLength; import static org.elasticsearch.test.ESTestCase.randomFrom; /** @@ -48,7 +47,9 @@ public static QueryBuilder createQuery(Random r) { case 1: return new TermQueryBuilderTests().createTestQueryBuilder(); case 2: - return new ExistsQueryBuilder(randomAlphaOfLength(5)); + // We make sure this query has no types to avoid deprecation warnings in the + // tests that use this method. + return new IdsQueryBuilderTests().createTestQueryBuilder().types(new String[0]); case 3: return createMultiTermQuery(r); default: From 932e250d0b2fa55fa0bb513ec4ede827e03164cf Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Mon, 7 Jan 2019 11:04:29 -0800 Subject: [PATCH 05/14] Avoid introducing a special case in TermsLookup#hashCode. --- .../main/java/org/elasticsearch/indices/TermsLookup.java | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/indices/TermsLookup.java b/server/src/main/java/org/elasticsearch/indices/TermsLookup.java index 31a1afdcf6d56..e0d8a581533a9 100644 --- a/server/src/main/java/org/elasticsearch/indices/TermsLookup.java +++ b/server/src/main/java/org/elasticsearch/indices/TermsLookup.java @@ -205,11 +205,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws @Override public int hashCode() { - if (type == null) { - return Objects.hash(index, id, path, routing); - } else { - return Objects.hash(index, type, id, path, routing); - } + return Objects.hash(index, type, id, path, routing); } @Override From 6678b83f4fdccd4eca3263c6af1b753df2ae6e5a Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Mon, 7 Jan 2019 11:08:17 -0800 Subject: [PATCH 06/14] Add some clarifying comments. --- server/src/main/java/org/elasticsearch/indices/TermsLookup.java | 1 + .../org/elasticsearch/index/query/TermsQueryBuilderTests.java | 2 ++ 2 files changed, 3 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/indices/TermsLookup.java b/server/src/main/java/org/elasticsearch/indices/TermsLookup.java index e0d8a581533a9..8d38724327b61 100644 --- a/server/src/main/java/org/elasticsearch/indices/TermsLookup.java +++ b/server/src/main/java/org/elasticsearch/indices/TermsLookup.java @@ -75,6 +75,7 @@ public TermsLookup(StreamInput in) throws IOException { if (in.getVersion().onOrAfter(Version.V_7_0_0)) { type = in.readOptionalString(); } else { + // Before 7.0, the type parameter was always non-null and serialized as a (non-optional) string. type = in.readString(); } id = in.readString(); diff --git a/server/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java index 5e66cd6f9c01c..9c1e9cdbd3ef8 100644 --- a/server/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java @@ -99,6 +99,8 @@ protected TermsQueryBuilder doCreateTestQueryBuilder() { } private TermsLookup randomTermsLookup() { + // Randomly choose between a typeless terms lookup and one with an explicit type to make sure we are + // testing both cases. TermsLookup lookup = randomBoolean() ? new TermsLookup(randomAlphaOfLength(10), randomAlphaOfLength(10), termsPath) : new TermsLookup(randomAlphaOfLength(10), randomAlphaOfLength(10), randomAlphaOfLength(10), termsPath); From e17610152a02092f43d44f402976bc1bb3e0a7f5 Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Mon, 7 Jan 2019 12:50:38 -0800 Subject: [PATCH 07/14] Prefer to use Strings.EMPTY_ARRAY. --- .../java/org/elasticsearch/index/query/RandomQueryBuilder.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/index/query/RandomQueryBuilder.java b/server/src/test/java/org/elasticsearch/index/query/RandomQueryBuilder.java index 8eb9ee1af0c0b..04d2d2c347bbf 100644 --- a/server/src/test/java/org/elasticsearch/index/query/RandomQueryBuilder.java +++ b/server/src/test/java/org/elasticsearch/index/query/RandomQueryBuilder.java @@ -21,6 +21,7 @@ import com.carrotsearch.randomizedtesting.generators.RandomNumbers; import com.carrotsearch.randomizedtesting.generators.RandomStrings; +import org.elasticsearch.common.Strings; import java.util.Random; @@ -49,7 +50,7 @@ public static QueryBuilder createQuery(Random r) { case 2: // We make sure this query has no types to avoid deprecation warnings in the // tests that use this method. - return new IdsQueryBuilderTests().createTestQueryBuilder().types(new String[0]); + return new IdsQueryBuilderTests().createTestQueryBuilder().types(Strings.EMPTY_ARRAY); case 3: return createMultiTermQuery(r); default: From 7a6100a52ccfb97a6b7afb2e3ab6f0b11136b31b Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Mon, 7 Jan 2019 13:01:53 -0800 Subject: [PATCH 08/14] Give a clear error message when typeless terms lookup queries are used with pre-7.0 nodes. --- .../elasticsearch/indices/TermsLookup.java | 5 +++ .../indices/TermsLookupTests.java | 31 +++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/indices/TermsLookup.java b/server/src/main/java/org/elasticsearch/indices/TermsLookup.java index 8d38724327b61..f8f9766037134 100644 --- a/server/src/main/java/org/elasticsearch/indices/TermsLookup.java +++ b/server/src/main/java/org/elasticsearch/indices/TermsLookup.java @@ -96,6 +96,11 @@ public void writeTo(StreamOutput out) throws IOException { if (out.getVersion().onOrAfter(Version.V_7_0_0)) { out.writeOptionalString(type); } else { + if (type == null) { + throw new IllegalArgumentException("Typeless [terms] lookup queries are not supported if any " + + "node is running a version lower than 7.0."); + + } out.writeString(type); } out.writeString(id); diff --git a/server/src/test/java/org/elasticsearch/indices/TermsLookupTests.java b/server/src/test/java/org/elasticsearch/indices/TermsLookupTests.java index ecb169ddaf7dc..5e4425fa6e68e 100644 --- a/server/src/test/java/org/elasticsearch/indices/TermsLookupTests.java +++ b/server/src/test/java/org/elasticsearch/indices/TermsLookupTests.java @@ -19,9 +19,11 @@ package org.elasticsearch.indices; +import org.elasticsearch.Version; import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.VersionUtils; import java.io.IOException; @@ -82,9 +84,38 @@ public void testSerialization() throws IOException { assertNotSame(deserializedLookup, termsLookup); } } + + try (BytesStreamOutput output = new BytesStreamOutput()) { + output.setVersion(Version.V_6_7_0); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> termsLookup.writeTo(output)); + assertEquals("Typeless [terms] lookup queries are not supported if any " + + "node is running a version lower than 7.0.", e.getMessage()); + } + } + + + public void testSerializationWithTypes() throws IOException { + TermsLookup termsLookup = randomTermsLookupWithTypes(); + try (BytesStreamOutput output = new BytesStreamOutput()) { + termsLookup.writeTo(output); + try (StreamInput in = output.bytes().streamInput()) { + TermsLookup deserializedLookup = new TermsLookup(in); + assertEquals(deserializedLookup, termsLookup); + assertEquals(deserializedLookup.hashCode(), termsLookup.hashCode()); + assertNotSame(deserializedLookup, termsLookup); + } + } } public static TermsLookup randomTermsLookup() { + return new TermsLookup( + randomAlphaOfLength(10), + randomAlphaOfLength(10), + randomAlphaOfLength(10).replace('.', '_') + ).routing(randomBoolean() ? randomAlphaOfLength(10) : null); + } + + public static TermsLookup randomTermsLookupWithTypes() { return new TermsLookup( randomAlphaOfLength(10), randomAlphaOfLength(10), From 12f5eaf5b2b96aafdbf949f1d7f23e6a5ab66c9b Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Mon, 7 Jan 2019 13:17:01 -0800 Subject: [PATCH 09/14] Minor spacing issue. --- .../org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java index be551cf4c1457..681f5a72a2e19 100644 --- a/server/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java @@ -192,7 +192,7 @@ public Item() { * @param index the index where the document is located * @param id and its id */ - public Item(@Nullable String index,String id) { + public Item(@Nullable String index, String id) { if (id == null) { throw new IllegalArgumentException("Item requires id to be non-null"); } From f3031ba5cc826ab30252f37b51f88a2afb1c29bd Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Mon, 7 Jan 2019 14:03:20 -0800 Subject: [PATCH 10/14] Test terms lookup serialization to 6.x nodes. --- .../src/main/java/org/elasticsearch/indices/TermsLookup.java | 2 +- .../test/java/org/elasticsearch/indices/TermsLookupTests.java | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/indices/TermsLookup.java b/server/src/main/java/org/elasticsearch/indices/TermsLookup.java index f8f9766037134..077116e2fd54e 100644 --- a/server/src/main/java/org/elasticsearch/indices/TermsLookup.java +++ b/server/src/main/java/org/elasticsearch/indices/TermsLookup.java @@ -98,7 +98,7 @@ public void writeTo(StreamOutput out) throws IOException { } else { if (type == null) { throw new IllegalArgumentException("Typeless [terms] lookup queries are not supported if any " + - "node is running a version lower than 7.0."); + "node is running a version before 7.0."); } out.writeString(type); diff --git a/server/src/test/java/org/elasticsearch/indices/TermsLookupTests.java b/server/src/test/java/org/elasticsearch/indices/TermsLookupTests.java index 5e4425fa6e68e..a3743f55d028b 100644 --- a/server/src/test/java/org/elasticsearch/indices/TermsLookupTests.java +++ b/server/src/test/java/org/elasticsearch/indices/TermsLookupTests.java @@ -23,7 +23,6 @@ import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.test.VersionUtils; import java.io.IOException; @@ -89,11 +88,10 @@ public void testSerialization() throws IOException { output.setVersion(Version.V_6_7_0); IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> termsLookup.writeTo(output)); assertEquals("Typeless [terms] lookup queries are not supported if any " + - "node is running a version lower than 7.0.", e.getMessage()); + "node is running a version before 7.0.", e.getMessage()); } } - public void testSerializationWithTypes() throws IOException { TermsLookup termsLookup = randomTermsLookupWithTypes(); try (BytesStreamOutput output = new BytesStreamOutput()) { From c9f48a3a5f99519a36558487170a0800043f139d Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Mon, 7 Jan 2019 14:05:39 -0800 Subject: [PATCH 11/14] Make sure GeoShapeQueryBuilderTests covers typeless queries. --- .../elasticsearch/index/query/GeoShapeQueryBuilderTests.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTests.java index 13a5594c49366..d44404e9a22d4 100644 --- a/server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTests.java @@ -96,7 +96,7 @@ protected GeoShapeQueryBuilder doCreateTestQueryBuilder(boolean indexedShape) { } else { indexedShapeToReturn = shape; indexedShapeId = randomAlphaOfLengthBetween(3, 20); - indexedShapeType = randomAlphaOfLengthBetween(3, 20); + indexedShapeType = randomBoolean() ? randomAlphaOfLengthBetween(3, 20) : null; builder = new GeoShapeQueryBuilder(fieldName(), indexedShapeId, indexedShapeType); if (randomBoolean()) { indexedShapeIndex = randomAlphaOfLengthBetween(3, 20); @@ -138,6 +138,7 @@ protected GetResponse executeGet(GetRequest getRequest) { String expectedShapeIndex = indexedShapeIndex == null ? GeoShapeQueryBuilder.DEFAULT_SHAPE_INDEX_NAME : indexedShapeIndex; assertThat(getRequest.index(), equalTo(expectedShapeIndex)); String expectedShapePath = indexedShapePath == null ? GeoShapeQueryBuilder.DEFAULT_SHAPE_FIELD_NAME : indexedShapePath; + String json; try { XContentBuilder builder = XContentFactory.jsonBuilder().prettyPrint(); From 806e957be9831d436885a6f4b8ad68bbc6600fdb Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Mon, 7 Jan 2019 14:44:40 -0800 Subject: [PATCH 12/14] Simplify the handling around null types for more_like_this queries. --- .../index/query/MoreLikeThisQueryBuilder.java | 16 ++++++++++------ .../search/morelikethis/MoreLikeThisIT.java | 2 +- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java index 681f5a72a2e19..93c88b18d9bc6 100644 --- a/server/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java @@ -55,6 +55,7 @@ import org.elasticsearch.index.mapper.IdFieldMapper; import org.elasticsearch.index.mapper.KeywordFieldMapper.KeywordFieldType; import org.elasticsearch.index.mapper.MappedFieldType; +import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.mapper.TextFieldMapper.TextFieldType; import java.io.IOException; @@ -300,10 +301,18 @@ public Item index(String index) { return this; } + /** + * @deprecated Types are in the process of being removed. + */ + @Deprecated public String type() { return type; } + /** + * @deprecated Types are in the process of being removed. + */ + @Deprecated public Item type(String type) { this.type = type; return this; @@ -1117,12 +1126,7 @@ private static void setDefaultIndexTypeFields(QueryShardContext context, Item it item.index(context.index().getName()); } if (item.type() == null) { - if (context.queryTypes().size() > 1) { - throw new QueryShardException(context, - "ambiguous type for item with id: " + item.id() + " and index: " + item.index()); - } else { - item.type(context.queryTypes().iterator().next()); - } + item.type(MapperService.SINGLE_MAPPING_NAME); } // default fields if not present but don't override for artificial docs if ((item.fields() == null || item.fields().length == 0) && item.doc() == null) { diff --git a/server/src/test/java/org/elasticsearch/search/morelikethis/MoreLikeThisIT.java b/server/src/test/java/org/elasticsearch/search/morelikethis/MoreLikeThisIT.java index 36eabf8d6ff68..2e29c7c5a3815 100644 --- a/server/src/test/java/org/elasticsearch/search/morelikethis/MoreLikeThisIT.java +++ b/server/src/test/java/org/elasticsearch/search/morelikethis/MoreLikeThisIT.java @@ -753,7 +753,7 @@ public void testWithMissingRouting() throws IOException { Throwable cause = exception.getCause(); assertThat(cause, instanceOf(RoutingMissingException.class)); - assertThat(cause.getMessage(), equalTo("routing is required for [test]/[type1]/[1]")); + assertThat(cause.getMessage(), equalTo("routing is required for [test]/[_doc]/[1]")); } { From 13c4854c3e369d764d5c987119b949899b36fdd7 Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Tue, 8 Jan 2019 10:21:57 -0800 Subject: [PATCH 13/14] Small refactors in response to review comments. --- .../index/query/GeoShapeQueryBuilder.java | 2 +- .../index/query/MoreLikeThisQueryBuilder.java | 2 +- .../index/query/TermsQueryBuilder.java | 14 ++++++++++---- .../index/query/MoreLikeThisQueryBuilderTests.java | 2 +- .../index/query/TermsQueryBuilderTests.java | 2 +- 5 files changed, 14 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java index 76bd81d6f9ef3..f8ffcfdc05bcc 100644 --- a/server/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java @@ -162,7 +162,7 @@ private GeoShapeQueryBuilder(String fieldName, ShapeBuilder shape, String indexe throw new IllegalArgumentException("fieldName is required"); } if (shape == null && indexedShapeId == null) { - throw new IllegalArgumentException("either shapeBytes or indexedShapeId is required"); + throw new IllegalArgumentException("either shape or indexedShapeId is required"); } this.fieldName = fieldName; this.shape = shape; diff --git a/server/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java index 93c88b18d9bc6..b90a1e60ffa0b 100644 --- a/server/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java @@ -965,7 +965,7 @@ public static MoreLikeThisQueryBuilder fromXContent(XContentParser parser) throw moreLikeThisQueryBuilder.stopWords(stopWords); } - if (!moreLikeThisQueryBuilder.isTypeless()) { + if (moreLikeThisQueryBuilder.isTypeless() == false) { deprecationLogger.deprecatedAndMaybeLog("more_like_this_query_with_types", TYPES_DEPRECATION_MESSAGE); } return moreLikeThisQueryBuilder; diff --git a/server/src/main/java/org/elasticsearch/index/query/TermsQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/TermsQueryBuilder.java index ee3192f02e0ae..ae7bbae63018b 100644 --- a/server/src/main/java/org/elasticsearch/index/query/TermsQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/TermsQueryBuilder.java @@ -218,6 +218,10 @@ public TermsLookup termsLookup() { return this.termsLookup; } + public boolean isTypeless() { + return termsLookup == null || termsLookup.type() == null; + } + private static final Set> INTEGER_TYPES = new HashSet<>( Arrays.asList(Byte.class, Short.class, Integer.class, Long.class)); private static final Set> STRING_TYPES = new HashSet<>( @@ -399,13 +403,15 @@ public static TermsQueryBuilder fromXContent(XContentParser parser) throws IOExc "followed by array of terms or a document lookup specification"); } - if (termsLookup != null && termsLookup.type() != null) { + TermsQueryBuilder builder = new TermsQueryBuilder(fieldName, values, termsLookup) + .boost(boost) + .queryName(queryName); + + if (builder.isTypeless() == false) { deprecationLogger.deprecatedAndMaybeLog("terms_lookup_with_types", TYPES_DEPRECATION_MESSAGE); } - return new TermsQueryBuilder(fieldName, values, termsLookup) - .boost(boost) - .queryName(queryName); + return builder; } static List parseValues(XContentParser parser) throws IOException { diff --git a/server/src/test/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilderTests.java index ead5b6b5f5e71..62613139b50fd 100644 --- a/server/src/test/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilderTests.java @@ -385,7 +385,7 @@ protected QueryBuilder parseQuery(XContentParser parser) throws IOException { assertThat(query, instanceOf(MoreLikeThisQueryBuilder.class)); MoreLikeThisQueryBuilder mltQuery = (MoreLikeThisQueryBuilder) query; - if (!mltQuery.isTypeless()) { + if (mltQuery.isTypeless() == false) { assertWarnings(MoreLikeThisQueryBuilder.TYPES_DEPRECATION_MESSAGE); } return query; diff --git a/server/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java index 9c1e9cdbd3ef8..d1e0de67369dc 100644 --- a/server/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java @@ -330,7 +330,7 @@ protected QueryBuilder parseQuery(XContentParser parser) throws IOException { assertThat(query, CoreMatchers.instanceOf(TermsQueryBuilder.class)); TermsQueryBuilder termsQuery = (TermsQueryBuilder) query; - if (termsQuery.termsLookup() != null && termsQuery.termsLookup().type() != null) { + if (termsQuery.isTypeless() == false) { assertWarnings(TermsQueryBuilder.TYPES_DEPRECATION_MESSAGE); } return query; From 0bdbd71a1721bc24c49d87a55ce9e96113820080 Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Tue, 8 Jan 2019 13:43:56 -0800 Subject: [PATCH 14/14] Fix an outdated message in GeoShapeQueryBuilderTests. --- .../elasticsearch/index/query/GeoShapeQueryBuilderTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTests.java index d44404e9a22d4..22f9705dcc5f9 100644 --- a/server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/GeoShapeQueryBuilderTests.java @@ -186,7 +186,7 @@ public void testNoShape() throws IOException { public void testNoIndexedShape() throws IOException { IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> new GeoShapeQueryBuilder(fieldName(), null, "type")); - assertEquals("either shapeBytes or indexedShapeId is required", e.getMessage()); + assertEquals("either shape or indexedShapeId is required", e.getMessage()); } public void testNoRelation() throws IOException {