From 9731ba4338f7fdd2e883cc721df06130aa250d6d Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Wed, 13 Mar 2019 15:04:41 +0100 Subject: [PATCH] Make the `type` parameter optional when percolating existing documents. (#39987) (#39989) `document_type` is the type to use for parsing the document to percolate, which is already optional and deprecated. However `percotale` queries also have the ability to percolate existing documents, identified by an index, an id and a type. This change makes the latter optional and deprecated. Closes #39963 --- .../query-dsl/percolate-query.asciidoc | 5 +- .../percolator/PercolateQueryBuilder.java | 27 ++++-- .../PercolateQueryBuilderTests.java | 57 ++++++++--- .../resources/rest-api-spec/test/10_basic.yml | 44 +++++++++ .../test/11_basic_with_types.yml | 96 +++++++++++++++++++ 5 files changed, 202 insertions(+), 27 deletions(-) create mode 100644 modules/percolator/src/test/resources/rest-api-spec/test/11_basic_with_types.yml diff --git a/docs/reference/query-dsl/percolate-query.asciidoc b/docs/reference/query-dsl/percolate-query.asciidoc index 89264af0f26..6444bdb743e 100644 --- a/docs/reference/query-dsl/percolate-query.asciidoc +++ b/docs/reference/query-dsl/percolate-query.asciidoc @@ -134,7 +134,7 @@ The following parameters are required when percolating a document: This is an optional parameter. `document`:: The source of the document being percolated. `documents`:: Like the `document` parameter, but accepts multiple documents via a json array. -`document_type`:: The type / mapping of the document being percolated. This setting is deprecated and only required for indices created before 6.0 +`document_type`:: The type / mapping of the document being percolated. This parameter is deprecated and will be removed in Elasticsearch 8.0. Instead of specifying the source of the document being percolated, the source can also be retrieved from an already stored document. The `percolate` query will then internally execute a get request to fetch that document. @@ -143,7 +143,7 @@ In that case the `document` parameter can be substituted with the following para [horizontal] `index`:: The index the document resides in. This is a required parameter. -`type`:: The type of the document to fetch. This is a required parameter. +`type`:: The type of the document to fetch. This parameter is deprecated and will be removed in Elasticsearch 8.0. `id`:: The id of the document to fetch. This is a required parameter. `routing`:: Optionally, routing to be used to fetch document to percolate. `preference`:: Optionally, preference to be used to fetch document to percolate. @@ -323,7 +323,6 @@ GET /my-index/_search "percolate" : { "field": "query", "index" : "my-index", - "type" : "_doc", "id" : "2", "version" : 1 <1> } 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 2cb37cb794d..44200823b6d 100644 --- a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java +++ b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java @@ -96,6 +96,10 @@ public class PercolateQueryBuilder extends AbstractQueryBuilder { if (getResponse.isExists() == false) { throw new ResourceNotFoundException( - "indexed document [{}/{}/{}] couldn't be found", indexedDocumentIndex, indexedDocumentType, indexedDocumentId + "indexed document [{}{}/{}] couldn't be found", indexedDocumentIndex, + indexedDocumentType == null ? "" : "/" + indexedDocumentType, indexedDocumentId ); } if(getResponse.isSourceEmpty()) { throw new IllegalArgumentException( - "indexed document [" + indexedDocumentIndex + "/" + indexedDocumentType + "/" + indexedDocumentId - + "] source disabled" + "indexed document [" + indexedDocumentIndex + (indexedDocumentType == null ? "" : "/" + indexedDocumentType) + + "/" + indexedDocumentId + "] source disabled" ); } documentSupplier.set(getResponse.getSourceAsBytesRef()); @@ -554,7 +563,7 @@ public class PercolateQueryBuilder extends AbstractQueryBuilder rewriteAndFetch(pqb, createShardContext())); - String expectedString = "indexed document [" + indexedDocumentIndex + "/" + indexedDocumentType + "/" + + String expectedString = "indexed document [" + indexedDocumentIndex + "/" + indexedDocumentId + "] couldn't be found"; assertThat(e.getMessage() , equalTo(expectedString)); } @@ -220,11 +219,6 @@ public class PercolateQueryBuilderTests extends AbstractQueryTestCase { - new PercolateQueryBuilder("_field", "_document_type", "_index", null, "_id", null, null, null); - }); - assertThat(e.getMessage(), equalTo("[type] is a required argument")); - e = expectThrows(IllegalArgumentException.class, () -> { new PercolateQueryBuilder("_field", "_document_type", "_index", "_type", null, null, null, null); }); @@ -237,6 +231,39 @@ public class PercolateQueryBuilderTests extends AbstractQueryTestCase())); + + QueryShardContext queryShardContext = createShardContext(); + QueryBuilder queryBuilder = parseQuery("{\"percolate\" : { \"index\": \"" + indexedDocumentIndex + "\", \"id\": \"" + + indexedDocumentId + "\", \"field\":\"" + queryField + "\"}}"); + 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 + "\"}}")); diff --git a/modules/percolator/src/test/resources/rest-api-spec/test/10_basic.yml b/modules/percolator/src/test/resources/rest-api-spec/test/10_basic.yml index 3ed2ed64d78..08d344687ad 100644 --- a/modules/percolator/src/test/resources/rest-api-spec/test/10_basic.yml +++ b/modules/percolator/src/test/resources/rest-api-spec/test/10_basic.yml @@ -1,5 +1,10 @@ --- "Test percolator basics via rest": + + - skip: + version: " - 6.99.99" + reason: types are required in requests before 7.0.0 + - do: indices.create: index: queries_index @@ -11,6 +16,15 @@ foo: type: keyword + - do: + indices.create: + index: documents_index + body: + mappings: + properties: + foo: + type: keyword + - do: index: index: queries_index @@ -19,6 +33,13 @@ query: match_all: {} + - do: + index: + index: documents_index + id: some_id + body: + foo: bar + - do: indices.refresh: {} @@ -44,3 +65,26 @@ 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 + 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 + id: some_id + - match: { responses.0.hits.total: 1 } 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 new file mode 100644 index 00000000000..896d2d514bc --- /dev/null +++ b/modules/percolator/src/test/resources/rest-api-spec/test/11_basic_with_types.yml @@ -0,0 +1,96 @@ +--- +"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 }