From 34947183a9cc6064deb69bc8ad76ca591d42b190 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 23 Sep 2019 22:00:33 +0100 Subject: [PATCH 1/6] Remove types from percolate query API --- .../percolator/PercolateQueryBuilder.java | 136 ++++-------------- .../PercolateQueryBuilderTests.java | 38 ++--- .../percolator/PercolatorQuerySearchIT.java | 12 +- 3 files changed, 45 insertions(+), 141 deletions(-) diff --git a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java index 1a89752fe1d0d..f66868b07031c 100644 --- a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java +++ b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java @@ -19,7 +19,6 @@ package org.elasticsearch.percolator; -import org.apache.logging.log4j.LogManager; import org.apache.lucene.analysis.Analyzer; import org.apache.lucene.analysis.DelegatingAnalyzerWrapper; import org.apache.lucene.index.BinaryDocValues; @@ -54,7 +53,6 @@ import org.elasticsearch.common.io.stream.NamedWriteableRegistry; 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; @@ -92,19 +90,11 @@ public class PercolateQueryBuilder extends AbstractQueryBuilder { public static final String NAME = "percolate"; - private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(ParseField.class)); - static final String DOCUMENT_TYPE_DEPRECATION_MESSAGE = "[types removal] Types are deprecated in [percolate] queries. " + - "The [document_type] should no longer be specified."; - static final String TYPE_DEPRECATION_MESSAGE = "[types removal] Types are deprecated in [percolate] queries. " + - "The [type] of the indexed document should no longer be specified."; - static final ParseField DOCUMENT_FIELD = new ParseField("document"); static final ParseField DOCUMENTS_FIELD = new ParseField("documents"); private static final ParseField NAME_FIELD = new ParseField("name"); private static final ParseField QUERY_FIELD = new ParseField("field"); - private static final ParseField DOCUMENT_TYPE_FIELD = new ParseField("document_type"); private static final ParseField INDEXED_DOCUMENT_FIELD_INDEX = new ParseField("index"); - private static final ParseField INDEXED_DOCUMENT_FIELD_TYPE = new ParseField("type"); private static final ParseField INDEXED_DOCUMENT_FIELD_ID = new ParseField("id"); private static final ParseField INDEXED_DOCUMENT_FIELD_ROUTING = new ParseField("routing"); private static final ParseField INDEXED_DOCUMENT_FIELD_PREFERENCE = new ParseField("preference"); @@ -112,29 +102,16 @@ public class PercolateQueryBuilder extends AbstractQueryBuilder documents; private final XContentType documentXContentType; private final String indexedDocumentIndex; - @Deprecated - private final String indexedDocumentType; private final String indexedDocumentId; private final String indexedDocumentRouting; private final String indexedDocumentPreference; private final Long indexedDocumentVersion; private final Supplier documentSupplier; - /** - * @deprecated use {@link #PercolateQueryBuilder(String, BytesReference, XContentType)} with the document content - * type to avoid autodetection. - */ - @Deprecated - public PercolateQueryBuilder(String field, String documentType, BytesReference document) { - this(field, documentType, Collections.singletonList(document), XContentHelper.xContentType(document)); - } - /** * Creates a percolator query builder instance for percolating a provided document. * @@ -143,7 +120,11 @@ public PercolateQueryBuilder(String field, String documentType, BytesReference d * @param documentXContentType The content type of the binary blob containing the document to percolate */ public PercolateQueryBuilder(String field, BytesReference document, XContentType documentXContentType) { - this(field, null, Collections.singletonList(document), documentXContentType); + this(field, Collections.singletonList(document), documentXContentType); + } + + private PercolateQueryBuilder(String field, BytesReference document) { + this(field, document, XContentHelper.xContentType(document)); } /** @@ -154,11 +135,6 @@ public PercolateQueryBuilder(String field, BytesReference document, XContentType * @param documentXContentType The content type of the binary blob containing the document to percolate */ public PercolateQueryBuilder(String field, List documents, XContentType documentXContentType) { - this(field, null, documents, documentXContentType); - } - - @Deprecated - public PercolateQueryBuilder(String field, String documentType, List documents, XContentType documentXContentType) { if (field == null) { throw new IllegalArgumentException("[field] is a required argument"); } @@ -166,11 +142,9 @@ public PercolateQueryBuilder(String field, String documentType, List documentSupplier) { - if (field == null) { - throw new IllegalArgumentException("[field] is a required argument"); - } - this.field = field; - this.documentType = documentType; - this.documents = Collections.emptyList(); - this.documentXContentType = null; - this.documentSupplier = documentSupplier; - indexedDocumentIndex = null; - indexedDocumentType = null; - indexedDocumentId = null; - indexedDocumentRouting = null; - indexedDocumentPreference = null; - indexedDocumentVersion = null; - } - /** * Creates a percolator query builder instance for percolating a document in a remote index. * * @param field The field that contains the percolator query * @param indexedDocumentIndex The index containing the document to percolate - * @param indexedDocumentType The type containing the document to percolate * @param indexedDocumentId The id of the document to percolate * @param indexedDocumentRouting The routing value for the document to percolate * @param indexedDocumentPreference The preference to use when fetching the document to percolate * @param indexedDocumentVersion The expected version of the document to percolate */ - public PercolateQueryBuilder(String field, String indexedDocumentIndex, String indexedDocumentType, String indexedDocumentId, - String indexedDocumentRouting, String indexedDocumentPreference, Long indexedDocumentVersion) { - this(field, null, indexedDocumentIndex, indexedDocumentType, indexedDocumentId, indexedDocumentRouting, - indexedDocumentPreference, indexedDocumentVersion); - } - - @Deprecated - public PercolateQueryBuilder(String field, String documentType, String indexedDocumentIndex, - String indexedDocumentType, String indexedDocumentId, String indexedDocumentRouting, + public PercolateQueryBuilder(String field, String indexedDocumentIndex, + String indexedDocumentId, String indexedDocumentRouting, String indexedDocumentPreference, Long indexedDocumentVersion) { if (field == null) { throw new IllegalArgumentException("[field] is a required argument"); @@ -226,9 +175,7 @@ public PercolateQueryBuilder(String field, String documentType, String indexedDo throw new IllegalArgumentException("[id] is a required argument"); } this.field = field; - this.documentType = documentType; this.indexedDocumentIndex = indexedDocumentIndex; - this.indexedDocumentType = indexedDocumentType; this.indexedDocumentId = indexedDocumentId; this.indexedDocumentRouting = indexedDocumentRouting; this.indexedDocumentPreference = indexedDocumentPreference; @@ -245,9 +192,15 @@ public PercolateQueryBuilder(String field, String documentType, String indexedDo super(in); field = in.readString(); name = in.readOptionalString(); - documentType = in.readOptionalString(); + if (in.getVersion().before(Version.V_8_0_0)) { + String documentType = in.readOptionalString(); + assert MapperService.SINGLE_MAPPING_NAME.equals(documentType); + } indexedDocumentIndex = in.readOptionalString(); - indexedDocumentType = in.readOptionalString(); + if (in.getVersion().before(Version.V_8_0_0)) { + String indexedDocumentType = in.readOptionalString(); + assert MapperService.SINGLE_MAPPING_NAME.equals(indexedDocumentType); + } indexedDocumentId = in.readOptionalString(); indexedDocumentRouting = in.readOptionalString(); indexedDocumentPreference = in.readOptionalString(); @@ -281,9 +234,13 @@ protected void doWriteTo(StreamOutput out) throws IOException { } out.writeString(field); out.writeOptionalString(name); - out.writeOptionalString(documentType); + if (out.getVersion().before(Version.V_8_0_0)) { + out.writeOptionalString(MapperService.SINGLE_MAPPING_NAME); + } out.writeOptionalString(indexedDocumentIndex); - out.writeOptionalString(indexedDocumentType); + if (out.getVersion().before(Version.V_8_0_0)) { + out.writeOptionalString(MapperService.SINGLE_MAPPING_NAME); + } out.writeOptionalString(indexedDocumentId); out.writeOptionalString(indexedDocumentRouting); out.writeOptionalString(indexedDocumentPreference); @@ -305,7 +262,6 @@ protected void doWriteTo(StreamOutput out) throws IOException { @Override protected void doXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(NAME); - builder.field(DOCUMENT_TYPE_FIELD.getPreferredName(), documentType); builder.field(QUERY_FIELD.getPreferredName(), field); if (name != null) { builder.field(NAME_FIELD.getPreferredName(), name); @@ -321,13 +277,10 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep } builder.endArray(); } - if (indexedDocumentIndex != null || indexedDocumentType != null || indexedDocumentId != null) { + if (indexedDocumentIndex != null || indexedDocumentId != null) { if (indexedDocumentIndex != null) { builder.field(INDEXED_DOCUMENT_FIELD_INDEX.getPreferredName(), indexedDocumentIndex); } - if (indexedDocumentType != null) { - builder.field(INDEXED_DOCUMENT_FIELD_TYPE.getPreferredName(), indexedDocumentType); - } if (indexedDocumentId != null) { builder.field(INDEXED_DOCUMENT_FIELD_ID.getPreferredName(), indexedDocumentId); } @@ -350,10 +303,8 @@ public static PercolateQueryBuilder fromXContent(XContentParser parser) throws I String field = null; String name = null; - String documentType = null; String indexedDocumentIndex = null; - String indexedDocumentType = null; String indexedDocumentId = null; String indexedDocumentRouting = null; String indexedDocumentPreference = null; @@ -415,12 +366,8 @@ public static PercolateQueryBuilder fromXContent(XContentParser parser) throws I field = parser.text(); } else if (NAME_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { name = parser.textOrNull(); - } else if (DOCUMENT_TYPE_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { - documentType = parser.textOrNull(); } else if (INDEXED_DOCUMENT_FIELD_INDEX.match(currentFieldName, parser.getDeprecationHandler())) { indexedDocumentIndex = parser.text(); - } else if (INDEXED_DOCUMENT_FIELD_TYPE.match(currentFieldName, parser.getDeprecationHandler())) { - indexedDocumentType = parser.text(); } else if (INDEXED_DOCUMENT_FIELD_ID.match(currentFieldName, parser.getDeprecationHandler())) { indexedDocumentId = parser.text(); } else if (INDEXED_DOCUMENT_FIELD_ROUTING.match(currentFieldName, parser.getDeprecationHandler())) { @@ -445,10 +392,10 @@ public static PercolateQueryBuilder fromXContent(XContentParser parser) throws I PercolateQueryBuilder queryBuilder; if (documents.isEmpty() == false) { - queryBuilder = new PercolateQueryBuilder(field, documentType, documents, XContentType.JSON); + queryBuilder = new PercolateQueryBuilder(field, documents, XContentType.JSON); } else if (indexedDocumentId != null) { - queryBuilder = new PercolateQueryBuilder(field, documentType, indexedDocumentIndex, indexedDocumentType, - indexedDocumentId, indexedDocumentRouting, indexedDocumentPreference, indexedDocumentVersion); + queryBuilder = new PercolateQueryBuilder(field, indexedDocumentIndex, indexedDocumentId, indexedDocumentRouting, + indexedDocumentPreference, indexedDocumentVersion); } else { throw new IllegalArgumentException("[" + PercolateQueryBuilder.NAME + "] query, nothing to percolate"); } @@ -463,10 +410,8 @@ public static PercolateQueryBuilder fromXContent(XContentParser parser) throws I @Override protected boolean doEquals(PercolateQueryBuilder other) { return Objects.equals(field, other.field) - && Objects.equals(documentType, other.documentType) && Objects.equals(documents, other.documents) && Objects.equals(indexedDocumentIndex, other.indexedDocumentIndex) - && Objects.equals(indexedDocumentType, other.indexedDocumentType) && Objects.equals(documentSupplier, other.documentSupplier) && Objects.equals(indexedDocumentId, other.indexedDocumentId); @@ -474,7 +419,7 @@ protected boolean doEquals(PercolateQueryBuilder other) { @Override protected int doHashCode() { - return Objects.hash(field, documentType, documents, indexedDocumentIndex, indexedDocumentType, indexedDocumentId, documentSupplier); + return Objects.hash(field, documents, indexedDocumentIndex, indexedDocumentId, documentSupplier); } @Override @@ -491,7 +436,7 @@ protected QueryBuilder doRewrite(QueryRewriteContext queryShardContext) { if (source == null) { return this; // not executed yet } else { - PercolateQueryBuilder rewritten = new PercolateQueryBuilder(field, documentType, + PercolateQueryBuilder rewritten = new PercolateQueryBuilder(field, Collections.singletonList(source), XContentHelper.xContentType(source)); if (name != null) { rewritten.setName(name); @@ -499,13 +444,7 @@ protected QueryBuilder doRewrite(QueryRewriteContext queryShardContext) { return rewritten; } } - GetRequest getRequest; - if (indexedDocumentType != null) { - deprecationLogger.deprecatedAndMaybeLog("percolate_with_type", TYPE_DEPRECATION_MESSAGE); - getRequest = new GetRequest(indexedDocumentIndex, indexedDocumentId); - } else { - getRequest = new GetRequest(indexedDocumentIndex, indexedDocumentId); - } + GetRequest getRequest = new GetRequest(indexedDocumentIndex, indexedDocumentId); getRequest.preference("_local"); getRequest.routing(indexedDocumentRouting); getRequest.preference(indexedDocumentPreference); @@ -517,14 +456,12 @@ protected QueryBuilder doRewrite(QueryRewriteContext queryShardContext) { client.get(getRequest, ActionListener.wrap(getResponse -> { if (getResponse.isExists() == false) { throw new ResourceNotFoundException( - "indexed document [{}{}/{}] couldn't be found", indexedDocumentIndex, - indexedDocumentType == null ? "" : "/" + indexedDocumentType, indexedDocumentId + "indexed document [{}/{}] couldn't be found", indexedDocumentIndex, indexedDocumentId ); } if(getResponse.isSourceEmpty()) { throw new IllegalArgumentException( - "indexed document [" + indexedDocumentIndex + (indexedDocumentType == null ? "" : "/" + indexedDocumentType) + - "/" + indexedDocumentId + "] source disabled" + "indexed document [" + indexedDocumentIndex + "/" + indexedDocumentId + "] source disabled" ); } documentSupplier.set(getResponse.getSourceAsBytesRef()); @@ -532,7 +469,7 @@ protected QueryBuilder doRewrite(QueryRewriteContext queryShardContext) { }, listener::onFailure)); }); - PercolateQueryBuilder rewritten = new PercolateQueryBuilder(field, documentType, documentSupplier::get); + PercolateQueryBuilder rewritten = new PercolateQueryBuilder(field, documentSupplier.get()); if (name != null) { rewritten.setName(name); } @@ -566,13 +503,6 @@ protected Query doToQuery(QueryShardContext context) throws IOException { final DocumentMapper docMapper; final MapperService mapperService = context.getMapperService(); String type = mapperService.documentMapper().type(); - if (documentType != null) { - deprecationLogger.deprecatedAndMaybeLog("percolate_with_document_type", DOCUMENT_TYPE_DEPRECATION_MESSAGE); - if (documentType.equals(type) == false) { - throw new IllegalArgumentException("specified document_type [" + documentType + - "] is not equal to the actual type [" + type + "]"); - } - } docMapper = mapperService.documentMapper(type); for (BytesReference document : documents) { docs.add(docMapper.parse(new SourceToParse(context.index().getName(), type, "_temp_id", document, documentXContentType))); @@ -622,10 +552,6 @@ public String getField() { return field; } - public String getDocumentType() { - return documentType; - } - public List getDocuments() { return documents; } diff --git a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolateQueryBuilderTests.java b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolateQueryBuilderTests.java index 8c818c504dd60..c1d9e24401547 100644 --- a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolateQueryBuilderTests.java +++ b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolateQueryBuilderTests.java @@ -121,10 +121,10 @@ private PercolateQueryBuilder doCreateTestQueryBuilder(boolean indexedDocument) indexedDocumentRouting = randomAlphaOfLength(4); indexedDocumentPreference = randomAlphaOfLength(4); indexedDocumentVersion = (long) randomIntBetween(0, Integer.MAX_VALUE); - queryBuilder = new PercolateQueryBuilder(queryField, null, indexedDocumentIndex, null, indexedDocumentId, + queryBuilder = new PercolateQueryBuilder(queryField, indexedDocumentIndex, indexedDocumentId, indexedDocumentRouting, indexedDocumentPreference, indexedDocumentVersion); } else { - queryBuilder = new PercolateQueryBuilder(queryField, null, documentSource, XContentType.JSON); + queryBuilder = new PercolateQueryBuilder(queryField, documentSource, XContentType.JSON); } if (randomBoolean()) { queryBuilder.setName(randomAlphaOfLength(4)); @@ -166,7 +166,6 @@ protected GetResponse executeGet(GetRequest getRequest) { protected void doAssertLuceneQuery(PercolateQueryBuilder queryBuilder, Query query, QueryShardContext context) throws IOException { assertThat(query, Matchers.instanceOf(PercolateQuery.class)); PercolateQuery percolateQuery = (PercolateQuery) query; - assertNull(queryBuilder.getDocumentType()); assertThat(percolateQuery.getDocuments(), Matchers.equalTo(documentSource)); } @@ -177,7 +176,7 @@ public void testMustRewrite() throws IOException { assertThat(e.getMessage(), equalTo("query builder must be rewritten first")); QueryBuilder rewrite = rewriteAndFetch(pqb, createShardContext()); PercolateQueryBuilder geoShapeQueryBuilder = - new PercolateQueryBuilder(pqb.getField(), pqb.getDocumentType(), documentSource, XContentType.JSON); + new PercolateQueryBuilder(pqb.getField(), documentSource, XContentType.JSON); assertEquals(geoShapeQueryBuilder, rewrite); } @@ -205,21 +204,21 @@ public void testRequiredParameters() { assertThat(e.getMessage(), equalTo("[field] is a required argument")); e = expectThrows(IllegalArgumentException.class, - () -> new PercolateQueryBuilder("_field", "_document_type", null, null)); + () -> new PercolateQueryBuilder("_field", (List)null, XContentType.JSON)); assertThat(e.getMessage(), equalTo("[document] is a required argument")); e = expectThrows(IllegalArgumentException.class, () -> { - new PercolateQueryBuilder(null, null, "_index", "_type", "_id", null, null, null); + new PercolateQueryBuilder(null, "_index", "_id", null, null, null); }); assertThat(e.getMessage(), equalTo("[field] is a required argument")); e = expectThrows(IllegalArgumentException.class, () -> { - new PercolateQueryBuilder("_field", "_document_type", null, "_type", "_id", null, null, null); + new PercolateQueryBuilder("_field", null, "_id", null, null, null); }); assertThat(e.getMessage(), equalTo("[index] is a required argument")); e = expectThrows(IllegalArgumentException.class, () -> { - new PercolateQueryBuilder("_field", "_document_type", "_index", "_type", null, null, null, null); + new PercolateQueryBuilder("_field", "_index", null, null, null, null); }); assertThat(e.getMessage(), equalTo("[id] is a required argument")); } @@ -230,14 +229,6 @@ public void testFromJsonNoDocumentType() throws IOException { queryBuilder.toQuery(queryShardContext); } - public void testFromJsonWithDocumentType() throws IOException { - QueryShardContext queryShardContext = createShardContext(); - QueryBuilder queryBuilder = parseQuery("{\"percolate\" : { \"document\": {}, \"document_type\":\"" + docType + "\", \"field\":\"" + - queryField + "\"}}"); - queryBuilder.toQuery(queryShardContext); - assertWarnings(PercolateQueryBuilder.DOCUMENT_TYPE_DEPRECATION_MESSAGE); - } - public void testFromJsonNoType() throws IOException { indexedDocumentIndex = randomAlphaOfLength(4); indexedDocumentId = randomAlphaOfLength(4); @@ -250,19 +241,6 @@ public void testFromJsonNoType() throws IOException { rewriteAndFetch(queryBuilder, queryShardContext).toQuery(queryShardContext); } - public void testFromJsonWithType() throws IOException { - indexedDocumentIndex = randomAlphaOfLength(4); - indexedDocumentId = randomAlphaOfLength(4); - indexedDocumentVersion = Versions.MATCH_ANY; - documentSource = Collections.singletonList(randomSource(new HashSet<>())); - - QueryShardContext queryShardContext = createShardContext(); - QueryBuilder queryBuilder = parseQuery("{\"percolate\" : { \"index\": \"" + indexedDocumentIndex + - "\", \"type\": \"_doc\", \"id\": \"" + indexedDocumentId + "\", \"field\":\"" + queryField + "\"}}"); - rewriteAndFetch(queryBuilder, queryShardContext).toQuery(queryShardContext); - assertWarnings(PercolateQueryBuilder.TYPE_DEPRECATION_MESSAGE); - } - public void testBothDocumentAndDocumentsSpecified() throws IOException { expectThrows(IllegalArgumentException.class, () -> parseQuery("{\"percolate\" : { \"document\": {}, \"documents\": [{}, {}], \"field\":\"" + queryField + "\"}}")); @@ -354,7 +332,7 @@ public void testSettingNameWhileRewritingWhenDocumentSupplierAndSourceNotNull() Supplier supplier = () -> new BytesArray("{\"test\": \"test\"}"); String testName = "name1"; QueryShardContext shardContext = createShardContext(); - PercolateQueryBuilder percolateQueryBuilder = new PercolateQueryBuilder(queryField, null, supplier); + PercolateQueryBuilder percolateQueryBuilder = new PercolateQueryBuilder(queryField, supplier.get(), XContentType.JSON); percolateQueryBuilder.setName(testName); QueryBuilder rewrittenQueryBuilder = percolateQueryBuilder.doRewrite(shardContext); diff --git a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorQuerySearchIT.java b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorQuerySearchIT.java index 6f2db8f15343d..ed361eb858f57 100644 --- a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorQuerySearchIT.java +++ b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorQuerySearchIT.java @@ -303,14 +303,14 @@ public void testPercolatorQueryExistingDocument() throws Exception { logger.info("percolating empty doc"); SearchResponse response = client().prepareSearch() - .setQuery(new PercolateQueryBuilder("query", "test", "type", "1", null, null, null)) + .setQuery(new PercolateQueryBuilder("query", "test", "1", null, null, null)) .get(); assertHitCount(response, 1); assertThat(response.getHits().getAt(0).getId(), equalTo("1")); logger.info("percolating doc with 1 field"); response = client().prepareSearch() - .setQuery(new PercolateQueryBuilder("query", "test", "type", "5", null, null, null)) + .setQuery(new PercolateQueryBuilder("query", "test", "5", null, null, null)) .addSort("_id", SortOrder.ASC) .get(); assertHitCount(response, 2); @@ -319,7 +319,7 @@ public void testPercolatorQueryExistingDocument() throws Exception { logger.info("percolating doc with 2 fields"); response = client().prepareSearch() - .setQuery(new PercolateQueryBuilder("query", "test", "type", "6", null, null, null)) + .setQuery(new PercolateQueryBuilder("query", "test", "6", null, null, null)) .addSort("_id", SortOrder.ASC) .get(); assertHitCount(response, 3); @@ -343,7 +343,7 @@ public void testPercolatorQueryExistingDocumentSourceDisabled() throws Exception logger.info("percolating empty doc with source disabled"); IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> { client().prepareSearch() - .setQuery(new PercolateQueryBuilder("query", "test", "type", "1", null, null, null)) + .setQuery(new PercolateQueryBuilder("query", "test", "1", null, null, null)) .get(); }); assertThat(e.getMessage(), containsString("source disabled")); @@ -806,9 +806,9 @@ public void testPercolatorQueryViaMultiSearch() throws Exception { .setQuery(new PercolateQueryBuilder("query", BytesReference.bytes(jsonBuilder().startObject().field("field1", "d").endObject()), XContentType.JSON))) .add(client().prepareSearch("test") - .setQuery(new PercolateQueryBuilder("query", "test", "type", "5", null, null, null))) + .setQuery(new PercolateQueryBuilder("query", "test", "5", null, null, null))) .add(client().prepareSearch("test") // non existing doc, so error element - .setQuery(new PercolateQueryBuilder("query", "test", "type", "6", null, null, null))) + .setQuery(new PercolateQueryBuilder("query", "test", "6", null, null, null))) .get(); MultiSearchResponse.Item item = response.getResponses()[0]; From 02a40e3069acd19803273dc6ee9f21c0d75b22d1 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Tue, 24 Sep 2019 13:17:16 +0100 Subject: [PATCH 2/6] Fix rewrite --- .../percolator/PercolateQueryBuilder.java | 17 ++++++++++++++++- .../percolator/PercolateQueryBuilderTests.java | 2 +- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java index f66868b07031c..a19f705f1906a 100644 --- a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java +++ b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java @@ -185,6 +185,21 @@ public PercolateQueryBuilder(String field, String indexedDocumentIndex, this.documentSupplier = null; } + protected PercolateQueryBuilder(String field, Supplier documentSupplier) { + if (field == null) { + throw new IllegalArgumentException("[field] is a required argument"); + } + this.field = field; + this.documents = Collections.emptyList(); + this.documentXContentType = null; + this.documentSupplier = documentSupplier; + indexedDocumentIndex = null; + indexedDocumentId = null; + indexedDocumentRouting = null; + indexedDocumentPreference = null; + indexedDocumentVersion = null; + } + /** * Read from a stream. */ @@ -469,7 +484,7 @@ protected QueryBuilder doRewrite(QueryRewriteContext queryShardContext) { }, listener::onFailure)); }); - PercolateQueryBuilder rewritten = new PercolateQueryBuilder(field, documentSupplier.get()); + PercolateQueryBuilder rewritten = new PercolateQueryBuilder(field, documentSupplier::get); if (name != null) { rewritten.setName(name); } diff --git a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolateQueryBuilderTests.java b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolateQueryBuilderTests.java index c1d9e24401547..064fa262410ac 100644 --- a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolateQueryBuilderTests.java +++ b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolateQueryBuilderTests.java @@ -332,7 +332,7 @@ public void testSettingNameWhileRewritingWhenDocumentSupplierAndSourceNotNull() Supplier supplier = () -> new BytesArray("{\"test\": \"test\"}"); String testName = "name1"; QueryShardContext shardContext = createShardContext(); - PercolateQueryBuilder percolateQueryBuilder = new PercolateQueryBuilder(queryField, supplier.get(), XContentType.JSON); + PercolateQueryBuilder percolateQueryBuilder = new PercolateQueryBuilder(queryField, supplier::get); percolateQueryBuilder.setName(testName); QueryBuilder rewrittenQueryBuilder = percolateQueryBuilder.doRewrite(shardContext); From ad80f49a45fc841c18708b1a072140399ce27588 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Tue, 24 Sep 2019 14:41:48 +0100 Subject: [PATCH 3/6] tests --- .../percolator/PercolatorQuerySearchIT.java | 2 +- .../test/11_basic_with_types.yml | 96 ------------------- 2 files changed, 1 insertion(+), 97 deletions(-) delete mode 100644 modules/percolator/src/test/resources/rest-api-spec/test/11_basic_with_types.yml diff --git a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorQuerySearchIT.java b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorQuerySearchIT.java index ed361eb858f57..ad4027f6c66f0 100644 --- a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorQuerySearchIT.java +++ b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorQuerySearchIT.java @@ -839,7 +839,7 @@ public void testPercolatorQueryViaMultiSearch() throws Exception { item = response.getResponses()[5]; assertThat(item.getResponse(), nullValue()); assertThat(item.getFailureMessage(), notNullValue()); - assertThat(item.getFailureMessage(), containsString("[test/type/6] couldn't be found")); + assertThat(item.getFailureMessage(), containsString("[test/6] couldn't be found")); } } diff --git a/modules/percolator/src/test/resources/rest-api-spec/test/11_basic_with_types.yml b/modules/percolator/src/test/resources/rest-api-spec/test/11_basic_with_types.yml deleted file mode 100644 index 896d2d514bcb9..0000000000000 --- a/modules/percolator/src/test/resources/rest-api-spec/test/11_basic_with_types.yml +++ /dev/null @@ -1,96 +0,0 @@ ---- -"Test percolator basics via rest": - - - do: - indices.create: - include_type_name: true - index: queries_index - body: - mappings: - queries_type: - properties: - query: - type: percolator - foo: - type: keyword - - - do: - indices.create: - include_type_name: true - index: documents_index - body: - mappings: - documents_type: - properties: - foo: - type: keyword - - - do: - index: - index: queries_index - type: queries_type - id: test_percolator - body: - query: - match_all: {} - - - do: - index: - index: documents_index - type: documents_type - id: some_id - body: - foo: bar - - - do: - indices.refresh: {} - - - do: - search: - rest_total_hits_as_int: true - body: - - query: - percolate: - field: query - document: - document_type: queries_type - foo: bar - - match: { hits.total: 1 } - - - do: - msearch: - rest_total_hits_as_int: true - body: - - index: queries_index - - query: - percolate: - field: query - document_type: queries_type - document: - foo: bar - - match: { responses.0.hits.total: 1 } - - - do: - search: - rest_total_hits_as_int: true - body: - - query: - percolate: - field: query - index: documents_index - type: documents_type - id: some_id - - match: { hits.total: 1 } - - - do: - msearch: - rest_total_hits_as_int: true - body: - - index: queries_index - - query: - percolate: - field: query - index: documents_index - type: documents_type - id: some_id - - match: { responses.0.hits.total: 1 } From be3bc3a75aada4ec1426a10513a58e0ee656abe4 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Wed, 25 Sep 2019 16:24:19 +0100 Subject: [PATCH 4/6] bwc checks --- .../org/elasticsearch/percolator/PercolateQueryBuilder.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java index a19f705f1906a..f08b510ba7fa2 100644 --- a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java +++ b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java @@ -209,12 +209,12 @@ protected PercolateQueryBuilder(String field, Supplier documentS name = in.readOptionalString(); if (in.getVersion().before(Version.V_8_0_0)) { String documentType = in.readOptionalString(); - assert MapperService.SINGLE_MAPPING_NAME.equals(documentType); + assert documentType == null || MapperService.SINGLE_MAPPING_NAME.equals(documentType); } indexedDocumentIndex = in.readOptionalString(); if (in.getVersion().before(Version.V_8_0_0)) { String indexedDocumentType = in.readOptionalString(); - assert MapperService.SINGLE_MAPPING_NAME.equals(indexedDocumentType); + assert indexedDocumentType == null || MapperService.SINGLE_MAPPING_NAME.equals(indexedDocumentType); } indexedDocumentId = in.readOptionalString(); indexedDocumentRouting = in.readOptionalString(); From 3c3b176dea652703620545fca3ac4805a7898eb0 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Thu, 26 Sep 2019 09:26:04 +0100 Subject: [PATCH 5/6] feedback --- .../elasticsearch/percolator/PercolateQueryBuilder.java | 8 ++------ .../percolator/PercolateQueryBuilderTests.java | 2 +- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java index f08b510ba7fa2..209bc1b398b02 100644 --- a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java +++ b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java @@ -123,10 +123,6 @@ public PercolateQueryBuilder(String field, BytesReference document, XContentType this(field, Collections.singletonList(document), documentXContentType); } - private PercolateQueryBuilder(String field, BytesReference document) { - this(field, document, XContentHelper.xContentType(document)); - } - /** * Creates a percolator query builder instance for percolating a provided document. * @@ -209,12 +205,12 @@ protected PercolateQueryBuilder(String field, Supplier documentS name = in.readOptionalString(); if (in.getVersion().before(Version.V_8_0_0)) { String documentType = in.readOptionalString(); - assert documentType == null || MapperService.SINGLE_MAPPING_NAME.equals(documentType); + assert documentType == null; } indexedDocumentIndex = in.readOptionalString(); if (in.getVersion().before(Version.V_8_0_0)) { String indexedDocumentType = in.readOptionalString(); - assert indexedDocumentType == null || MapperService.SINGLE_MAPPING_NAME.equals(indexedDocumentType); + assert indexedDocumentType == null; } indexedDocumentId = in.readOptionalString(); indexedDocumentRouting = in.readOptionalString(); diff --git a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolateQueryBuilderTests.java b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolateQueryBuilderTests.java index 064fa262410ac..f78aad3d3c38a 100644 --- a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolateQueryBuilderTests.java +++ b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolateQueryBuilderTests.java @@ -332,7 +332,7 @@ public void testSettingNameWhileRewritingWhenDocumentSupplierAndSourceNotNull() Supplier supplier = () -> new BytesArray("{\"test\": \"test\"}"); String testName = "name1"; QueryShardContext shardContext = createShardContext(); - PercolateQueryBuilder percolateQueryBuilder = new PercolateQueryBuilder(queryField, supplier::get); + PercolateQueryBuilder percolateQueryBuilder = new PercolateQueryBuilder(queryField, supplier); percolateQueryBuilder.setName(testName); QueryBuilder rewrittenQueryBuilder = percolateQueryBuilder.doRewrite(shardContext); From 48faaadf0e83be24418114e915c54f65932e5638 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Fri, 27 Sep 2019 09:23:04 +0100 Subject: [PATCH 6/6] feedback --- .../org/elasticsearch/percolator/PercolateQueryBuilder.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java index 209bc1b398b02..be30086f9667a 100644 --- a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java +++ b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java @@ -246,11 +246,13 @@ protected void doWriteTo(StreamOutput out) throws IOException { out.writeString(field); out.writeOptionalString(name); if (out.getVersion().before(Version.V_8_0_0)) { - out.writeOptionalString(MapperService.SINGLE_MAPPING_NAME); + // In 7x, typeless percolate queries are represented by null documentType values + out.writeOptionalString(null); } out.writeOptionalString(indexedDocumentIndex); if (out.getVersion().before(Version.V_8_0_0)) { - out.writeOptionalString(MapperService.SINGLE_MAPPING_NAME); + // In 7x, typeless percolate queries are represented by null indexedDocumentType values + out.writeOptionalString(null); } out.writeOptionalString(indexedDocumentId); out.writeOptionalString(indexedDocumentRouting);