From fb2c8ad6ddcfe0be68e6a1a98ab3779a5c9e0570 Mon Sep 17 00:00:00 2001 From: bellengao Date: Sun, 1 Mar 2020 15:07:18 +0800 Subject: [PATCH 1/2] Fix inaccurate total hit count in _search/template api --- .../mustache/RestSearchTemplateAction.java | 1 - .../TransportSearchTemplateAction.java | 20 ++++- .../mustache/MultiSearchTemplateIT.java | 73 +++++++++++++++++++ .../script/mustache/SearchTemplateIT.java | 62 ++++++++++++++++ 4 files changed, 152 insertions(+), 4 deletions(-) diff --git a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestSearchTemplateAction.java b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestSearchTemplateAction.java index fe8e13d7ee239..dfc0f3d0228b2 100644 --- a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestSearchTemplateAction.java +++ b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/RestSearchTemplateAction.java @@ -72,7 +72,6 @@ public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client searchTemplateRequest = SearchTemplateRequest.fromXContent(parser); } searchTemplateRequest.setRequest(searchRequest); - RestSearchAction.checkRestTotalHits(request, searchRequest); return channel -> client.execute(SearchTemplateAction.INSTANCE, searchTemplateRequest, new RestStatusToXContentListener<>(channel)); } diff --git a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/TransportSearchTemplateAction.java b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/TransportSearchTemplateAction.java index a78cfbd2b6eec..e78879fac7978 100644 --- a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/TransportSearchTemplateAction.java +++ b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/TransportSearchTemplateAction.java @@ -32,15 +32,16 @@ import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.rest.action.search.RestSearchAction; import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptService; import org.elasticsearch.script.ScriptType; import org.elasticsearch.script.TemplateScript; import org.elasticsearch.search.builder.SearchSourceBuilder; +import org.elasticsearch.search.internal.SearchContext; import org.elasticsearch.tasks.Task; import org.elasticsearch.transport.TransportService; -import java.io.IOException; import java.util.Collections; public class TransportSearchTemplateAction extends HandledTransportAction { @@ -85,13 +86,13 @@ public void onFailure(Exception t) { } else { listener.onResponse(response); } - } catch (IOException e) { + } catch (Exception e) { listener.onFailure(e); } } static SearchRequest convert(SearchTemplateRequest searchTemplateRequest, SearchTemplateResponse response, ScriptService scriptService, - NamedXContentRegistry xContentRegistry) throws IOException { + NamedXContentRegistry xContentRegistry) throws Exception { Script script = new Script(searchTemplateRequest.getScriptType(), searchTemplateRequest.getScriptType() == ScriptType.STORED ? null : TEMPLATE_LANG, searchTemplateRequest.getScript(), searchTemplateRequest.getScriptParams() == null ? Collections.emptyMap() : searchTemplateRequest.getScriptParams()); @@ -110,6 +111,19 @@ static SearchRequest convert(SearchTemplateRequest searchTemplateRequest, Search builder.parseXContent(parser, false); builder.explain(searchTemplateRequest.isExplain()); builder.profile(searchTemplateRequest.isProfile()); + if (searchRequest.source() == null) { + searchRequest.source(new SearchSourceBuilder()); + } + Integer trackTotalHitsUpTo = searchRequest.source().trackTotalHitsUpTo(); + if (trackTotalHitsUpTo != null) { + if (builder.trackTotalHitsUpTo() == null) { + builder.trackTotalHitsUpTo(trackTotalHitsUpTo); + } else if (builder.trackTotalHitsUpTo() != SearchContext.TRACK_TOTAL_HITS_ACCURATE + && builder.trackTotalHitsUpTo() != SearchContext.TRACK_TOTAL_HITS_DISABLED) { + throw new IllegalArgumentException("[" + RestSearchAction.TOTAL_HITS_AS_INT_PARAM + "] cannot be used " + + "if the tracking of total hits is not accurate, got " + trackTotalHitsUpTo); + } + } searchRequest.source(builder); } return searchRequest; diff --git a/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/MultiSearchTemplateIT.java b/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/MultiSearchTemplateIT.java index f3570e0b80cec..452e18a1129b5 100644 --- a/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/MultiSearchTemplateIT.java +++ b/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/MultiSearchTemplateIT.java @@ -19,12 +19,15 @@ package org.elasticsearch.script.mustache; +import org.elasticsearch.action.bulk.BulkRequestBuilder; import org.elasticsearch.action.index.IndexRequestBuilder; import org.elasticsearch.action.search.SearchRequest; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.script.ScriptType; +import org.elasticsearch.search.builder.SearchSourceBuilder; import org.elasticsearch.test.ESIntegTestCase; import java.util.Arrays; @@ -36,6 +39,7 @@ import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; import static org.hamcrest.Matchers.arrayWithSize; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.instanceOf; @@ -173,4 +177,73 @@ public void testBasic() throws Exception { assertThat(searchTemplateResponse5.getSource().utf8ToString(), equalTo("{\"query\":{\"terms\":{\"group\":[1,2,3,]}}}")); } + + public void testMultiTemplateQueryHitCount() throws Exception { + BulkRequestBuilder bulkRequestBuilder = client().prepareBulk(); + for (int i =0; i <10001; i ++) { + bulkRequestBuilder.add(client().prepareIndex("msearch").setId(String.valueOf(i)).setSource("{\"theField\":\"foo\"}", + XContentType.JSON)); + } + bulkRequestBuilder.get(); + client().admin().indices().prepareRefresh().get(); + + MultiSearchTemplateRequest multiRequest = new MultiSearchTemplateRequest(); + + // Search #1 + SearchTemplateRequest search1 = new SearchTemplateRequest(); + search1.setRequest(new SearchRequest().indices("msearch")); + search1.setScriptType(ScriptType.INLINE); + search1.setScript("{{! ignore me }}{\"query\":{\"match_all\":{}}}"); + multiRequest.add(search1); + + // Search #2 + // When the request parameter `rest_total_hits_as_int` is set to true, `trackTotalHits` will also be set to true, + // we should test that we can get an accurate hits count. + SearchTemplateRequest search2 = new SearchTemplateRequest(); + SearchRequest searchRequestWithRestTotalHitsAsInt = new SearchRequest(); + searchRequestWithRestTotalHitsAsInt.indices("msearch"); + SearchSourceBuilder builder = new SearchSourceBuilder(); + searchRequestWithRestTotalHitsAsInt.source(builder.trackTotalHits(true)); + search2.setRequest(searchRequestWithRestTotalHitsAsInt); + search2.setScriptType(ScriptType.INLINE); + search2.setScript("{{! ignore me }}{\"query\":{\"match_all\":{}}}"); + multiRequest.add(search2); + + + // Search #3 + // When `rest_total_hits_as_int` is set to true, `trackTotalHits` should not be set to a not accurate number + SearchTemplateRequest search3 = new SearchTemplateRequest(); + search3.setRequest(searchRequestWithRestTotalHitsAsInt); + search3.setScriptType(ScriptType.INLINE); + search3.setScript("{{! ignore me }}{\"query\":{\"match_all\":{}}, \"track_total_hits\":100}"); + multiRequest.add(search3); + + // Search #4 + SearchTemplateRequest search4 = new SearchTemplateRequest(); + search4.setRequest(new SearchRequest().indices("msearch")); + search4.setScriptType(ScriptType.INLINE); + search4.setScript("{{! ignore me }}{\"query\":{\"match_all\":{}}, \"track_total_hits\":true}"); + multiRequest.add(search4); + + MultiSearchTemplateResponse response = client().execute(MultiSearchTemplateAction.INSTANCE, multiRequest).get(); + assertThat(response.getResponses(), arrayWithSize(4)); + assertThat(response.getTook().millis(), greaterThan(0L)); + + MultiSearchTemplateResponse.Item response1 = response.getResponses()[0]; + assertThat(response1.isFailure(), is(false)); + assertThat(response1.getResponse().getResponse().getHits().getTotalHits().value, equalTo(10000L)); + + MultiSearchTemplateResponse.Item response2 = response.getResponses()[1]; + assertThat(response2.isFailure(), is(false)); + assertThat(response2.getResponse().getResponse().getHits().getTotalHits().value, equalTo(10001L)); + + MultiSearchTemplateResponse.Item response3 = response.getResponses()[2]; + assertThat(response3.isFailure(), is(true)); + assertThat(response3.getFailureMessage(), containsString( + "[rest_total_hits_as_int] cannot be used if the tracking of total hits is not accurate")); + + MultiSearchTemplateResponse.Item response4 = response.getResponses()[3]; + assertThat(response4.isFailure(), is(false)); + assertThat(response4.getResponse().getResponse().getHits().getTotalHits().value, equalTo(10001L)); + } } diff --git a/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/SearchTemplateIT.java b/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/SearchTemplateIT.java index 12ab62c5a712b..fd9d59a183244 100644 --- a/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/SearchTemplateIT.java +++ b/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/SearchTemplateIT.java @@ -27,6 +27,7 @@ import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.script.ScriptType; +import org.elasticsearch.search.builder.SearchSourceBuilder; import org.elasticsearch.test.ESSingleNodeTestCase; import org.junit.Before; @@ -106,6 +107,67 @@ public void testTemplateQueryAsEscapedString() throws Exception { assertThat(searchResponse.getResponse().getHits().getHits().length, equalTo(1)); } + /** + * Test the template query's hit count with an index which has more than 10k documents, relates to #52801. + */ + public void testTemplateQueryHitCount() throws Exception { + BulkRequestBuilder bulkRequestBuilder = client().prepareBulk(); + for (int i =0; i <10001; i ++) { + bulkRequestBuilder.add(client().prepareIndex("test").setId(String.valueOf(i)).setSource("{\"theField\":\"foo\"}", + XContentType.JSON)); + } + bulkRequestBuilder.get(); + client().admin().indices().prepareRefresh().get(); + + SearchRequest searchRequest = new SearchRequest(); + searchRequest.indices("test"); + + String queryWithNoTrackTotalHits = "{\"source\" : \"{ \\\"query\\\":{\\\"match_all\\\":{}}}\"}"; + SearchTemplateRequest request = SearchTemplateRequest.fromXContent(createParser(JsonXContent.jsonXContent, + queryWithNoTrackTotalHits)); + request.setRequest(searchRequest); + SearchTemplateResponse searchResponse1 = client().execute(SearchTemplateAction.INSTANCE, request).get(); + assertThat(searchResponse1.getResponse().getHits().getTotalHits().value, equalTo(10000L)); + + // When the request parameter `rest_total_hits_as_int` is set to true, `trackTotalHits` will also be set to true, + // we should test that we can get an accurate hits count. + SearchSourceBuilder builder = new SearchSourceBuilder(); + searchRequest.source(builder.trackTotalHits(true)); + request.setRequest(searchRequest); + SearchTemplateResponse searchResponse2 = client().execute(SearchTemplateAction.INSTANCE, request).get(); + assertThat(searchResponse2.getResponse().getHits().getTotalHits().value, equalTo(10001L)); + + // When `rest_total_hits_as_int` is set to true, `trackTotalHits` should not be set to a not accurate number + String queryWithInvalidTrackTotalHits = + "{" + " \"source\" : \"{ \\\"track_total_hits\\\": \\\"{{trackTotalHits}}\\\", \\\"query\\\":{\\\"match_all\\\":{}}}\"," + + " \"params\":{" + + " \"trackTotalHits\": 100" + + " }" + + "}"; + SearchTemplateRequest requestWithInvalidTrackTotalHits = SearchTemplateRequest.fromXContent(createParser(JsonXContent.jsonXContent, + queryWithInvalidTrackTotalHits)); + requestWithInvalidTrackTotalHits.setRequest(searchRequest); + Exception e = expectThrows(Exception.class, + () -> client().execute(SearchTemplateAction.INSTANCE, requestWithInvalidTrackTotalHits).get()); + assertThat(e.getMessage(), containsString( + "[rest_total_hits_as_int] cannot be used if the tracking of total hits is not accurate")); + + // When `rest_total_hits_as_int` is not set but `trackTotalHits` is set to true, we can also get an accurate hits count + String queryWithValidTrackTotalHits = + "{" + " \"source\" : \"{ \\\"track_total_hits\\\": \\\"{{trackTotalHits}}\\\", \\\"query\\\":{\\\"match_all\\\":{}}}\"," + + " \"params\":{" + + " \"trackTotalHits\": true" + + " }" + + "}"; + SearchTemplateRequest requestWithValidTrackTotalHits = SearchTemplateRequest.fromXContent(createParser(JsonXContent.jsonXContent, + queryWithValidTrackTotalHits)); + SearchRequest newSearchRequest = new SearchRequest(); + newSearchRequest.indices("test"); + requestWithValidTrackTotalHits.setRequest(newSearchRequest); + SearchTemplateResponse searchResponse3 = client().execute(SearchTemplateAction.INSTANCE, requestWithValidTrackTotalHits).get(); + assertThat(searchResponse3.getResponse().getHits().getTotalHits().value, equalTo(10001L)); + } + /** * Test that template can contain conditional clause. In this case it is at * the beginning of the string. From c1372b1445c6ac0bb369b0b03d9fe65c578f8697 Mon Sep 17 00:00:00 2001 From: bellengao Date: Sat, 7 Mar 2020 19:30:16 +0800 Subject: [PATCH 2/2] use yaml test instead of the test code --- .../TransportSearchTemplateAction.java | 37 ++++++---- .../mustache/MultiSearchTemplateIT.java | 73 ------------------- .../script/mustache/SearchTemplateIT.java | 62 ---------------- .../test/lang_mustache/30_search_template.yml | 13 ++++ .../50_multi_search_template.yml | 8 ++ 5 files changed, 43 insertions(+), 150 deletions(-) diff --git a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/TransportSearchTemplateAction.java b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/TransportSearchTemplateAction.java index e78879fac7978..586deae9b7309 100644 --- a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/TransportSearchTemplateAction.java +++ b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/TransportSearchTemplateAction.java @@ -42,6 +42,7 @@ import org.elasticsearch.tasks.Task; import org.elasticsearch.transport.TransportService; +import java.io.IOException; import java.util.Collections; public class TransportSearchTemplateAction extends HandledTransportAction { @@ -86,13 +87,13 @@ public void onFailure(Exception t) { } else { listener.onResponse(response); } - } catch (Exception e) { + } catch (IOException e) { listener.onFailure(e); } } static SearchRequest convert(SearchTemplateRequest searchTemplateRequest, SearchTemplateResponse response, ScriptService scriptService, - NamedXContentRegistry xContentRegistry) throws Exception { + NamedXContentRegistry xContentRegistry) throws IOException { Script script = new Script(searchTemplateRequest.getScriptType(), searchTemplateRequest.getScriptType() == ScriptType.STORED ? null : TEMPLATE_LANG, searchTemplateRequest.getScript(), searchTemplateRequest.getScriptParams() == null ? Collections.emptyMap() : searchTemplateRequest.getScriptParams()); @@ -111,21 +112,27 @@ static SearchRequest convert(SearchTemplateRequest searchTemplateRequest, Search builder.parseXContent(parser, false); builder.explain(searchTemplateRequest.isExplain()); builder.profile(searchTemplateRequest.isProfile()); - if (searchRequest.source() == null) { - searchRequest.source(new SearchSourceBuilder()); - } - Integer trackTotalHitsUpTo = searchRequest.source().trackTotalHitsUpTo(); - if (trackTotalHitsUpTo != null) { - if (builder.trackTotalHitsUpTo() == null) { - builder.trackTotalHitsUpTo(trackTotalHitsUpTo); - } else if (builder.trackTotalHitsUpTo() != SearchContext.TRACK_TOTAL_HITS_ACCURATE - && builder.trackTotalHitsUpTo() != SearchContext.TRACK_TOTAL_HITS_DISABLED) { - throw new IllegalArgumentException("[" + RestSearchAction.TOTAL_HITS_AS_INT_PARAM + "] cannot be used " + - "if the tracking of total hits is not accurate, got " + trackTotalHitsUpTo); - } - } + checkRestTotalHitsAsInt(searchRequest, builder); searchRequest.source(builder); } return searchRequest; } + + private static void checkRestTotalHitsAsInt(SearchRequest searchRequest, SearchSourceBuilder searchSourceBuilder) { + if (searchRequest.source() == null) { + searchRequest.source(new SearchSourceBuilder()); + } + Integer trackTotalHitsUpTo = searchRequest.source().trackTotalHitsUpTo(); + // trackTotalHitsUpTo is set to Integer.MAX_VALUE when `rest_total_hits_as_int` is true + if (trackTotalHitsUpTo != null) { + if (searchSourceBuilder.trackTotalHitsUpTo() == null) { + // trackTotalHitsUpTo should be set here, ensure that we can get an accurate total hits count + searchSourceBuilder.trackTotalHitsUpTo(trackTotalHitsUpTo); + } else if (searchSourceBuilder.trackTotalHitsUpTo() != SearchContext.TRACK_TOTAL_HITS_ACCURATE + && searchSourceBuilder.trackTotalHitsUpTo() != SearchContext.TRACK_TOTAL_HITS_DISABLED) { + throw new IllegalArgumentException("[" + RestSearchAction.TOTAL_HITS_AS_INT_PARAM + "] cannot be used " + + "if the tracking of total hits is not accurate, got " + searchSourceBuilder.trackTotalHitsUpTo()); + } + } + } } diff --git a/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/MultiSearchTemplateIT.java b/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/MultiSearchTemplateIT.java index 452e18a1129b5..f3570e0b80cec 100644 --- a/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/MultiSearchTemplateIT.java +++ b/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/MultiSearchTemplateIT.java @@ -19,15 +19,12 @@ package org.elasticsearch.script.mustache; -import org.elasticsearch.action.bulk.BulkRequestBuilder; import org.elasticsearch.action.index.IndexRequestBuilder; import org.elasticsearch.action.search.SearchRequest; import org.elasticsearch.common.Strings; -import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.script.ScriptType; -import org.elasticsearch.search.builder.SearchSourceBuilder; import org.elasticsearch.test.ESIntegTestCase; import java.util.Arrays; @@ -39,7 +36,6 @@ import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; import static org.hamcrest.Matchers.arrayWithSize; -import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.instanceOf; @@ -177,73 +173,4 @@ public void testBasic() throws Exception { assertThat(searchTemplateResponse5.getSource().utf8ToString(), equalTo("{\"query\":{\"terms\":{\"group\":[1,2,3,]}}}")); } - - public void testMultiTemplateQueryHitCount() throws Exception { - BulkRequestBuilder bulkRequestBuilder = client().prepareBulk(); - for (int i =0; i <10001; i ++) { - bulkRequestBuilder.add(client().prepareIndex("msearch").setId(String.valueOf(i)).setSource("{\"theField\":\"foo\"}", - XContentType.JSON)); - } - bulkRequestBuilder.get(); - client().admin().indices().prepareRefresh().get(); - - MultiSearchTemplateRequest multiRequest = new MultiSearchTemplateRequest(); - - // Search #1 - SearchTemplateRequest search1 = new SearchTemplateRequest(); - search1.setRequest(new SearchRequest().indices("msearch")); - search1.setScriptType(ScriptType.INLINE); - search1.setScript("{{! ignore me }}{\"query\":{\"match_all\":{}}}"); - multiRequest.add(search1); - - // Search #2 - // When the request parameter `rest_total_hits_as_int` is set to true, `trackTotalHits` will also be set to true, - // we should test that we can get an accurate hits count. - SearchTemplateRequest search2 = new SearchTemplateRequest(); - SearchRequest searchRequestWithRestTotalHitsAsInt = new SearchRequest(); - searchRequestWithRestTotalHitsAsInt.indices("msearch"); - SearchSourceBuilder builder = new SearchSourceBuilder(); - searchRequestWithRestTotalHitsAsInt.source(builder.trackTotalHits(true)); - search2.setRequest(searchRequestWithRestTotalHitsAsInt); - search2.setScriptType(ScriptType.INLINE); - search2.setScript("{{! ignore me }}{\"query\":{\"match_all\":{}}}"); - multiRequest.add(search2); - - - // Search #3 - // When `rest_total_hits_as_int` is set to true, `trackTotalHits` should not be set to a not accurate number - SearchTemplateRequest search3 = new SearchTemplateRequest(); - search3.setRequest(searchRequestWithRestTotalHitsAsInt); - search3.setScriptType(ScriptType.INLINE); - search3.setScript("{{! ignore me }}{\"query\":{\"match_all\":{}}, \"track_total_hits\":100}"); - multiRequest.add(search3); - - // Search #4 - SearchTemplateRequest search4 = new SearchTemplateRequest(); - search4.setRequest(new SearchRequest().indices("msearch")); - search4.setScriptType(ScriptType.INLINE); - search4.setScript("{{! ignore me }}{\"query\":{\"match_all\":{}}, \"track_total_hits\":true}"); - multiRequest.add(search4); - - MultiSearchTemplateResponse response = client().execute(MultiSearchTemplateAction.INSTANCE, multiRequest).get(); - assertThat(response.getResponses(), arrayWithSize(4)); - assertThat(response.getTook().millis(), greaterThan(0L)); - - MultiSearchTemplateResponse.Item response1 = response.getResponses()[0]; - assertThat(response1.isFailure(), is(false)); - assertThat(response1.getResponse().getResponse().getHits().getTotalHits().value, equalTo(10000L)); - - MultiSearchTemplateResponse.Item response2 = response.getResponses()[1]; - assertThat(response2.isFailure(), is(false)); - assertThat(response2.getResponse().getResponse().getHits().getTotalHits().value, equalTo(10001L)); - - MultiSearchTemplateResponse.Item response3 = response.getResponses()[2]; - assertThat(response3.isFailure(), is(true)); - assertThat(response3.getFailureMessage(), containsString( - "[rest_total_hits_as_int] cannot be used if the tracking of total hits is not accurate")); - - MultiSearchTemplateResponse.Item response4 = response.getResponses()[3]; - assertThat(response4.isFailure(), is(false)); - assertThat(response4.getResponse().getResponse().getHits().getTotalHits().value, equalTo(10001L)); - } } diff --git a/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/SearchTemplateIT.java b/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/SearchTemplateIT.java index fd9d59a183244..12ab62c5a712b 100644 --- a/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/SearchTemplateIT.java +++ b/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/SearchTemplateIT.java @@ -27,7 +27,6 @@ import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.script.ScriptType; -import org.elasticsearch.search.builder.SearchSourceBuilder; import org.elasticsearch.test.ESSingleNodeTestCase; import org.junit.Before; @@ -107,67 +106,6 @@ public void testTemplateQueryAsEscapedString() throws Exception { assertThat(searchResponse.getResponse().getHits().getHits().length, equalTo(1)); } - /** - * Test the template query's hit count with an index which has more than 10k documents, relates to #52801. - */ - public void testTemplateQueryHitCount() throws Exception { - BulkRequestBuilder bulkRequestBuilder = client().prepareBulk(); - for (int i =0; i <10001; i ++) { - bulkRequestBuilder.add(client().prepareIndex("test").setId(String.valueOf(i)).setSource("{\"theField\":\"foo\"}", - XContentType.JSON)); - } - bulkRequestBuilder.get(); - client().admin().indices().prepareRefresh().get(); - - SearchRequest searchRequest = new SearchRequest(); - searchRequest.indices("test"); - - String queryWithNoTrackTotalHits = "{\"source\" : \"{ \\\"query\\\":{\\\"match_all\\\":{}}}\"}"; - SearchTemplateRequest request = SearchTemplateRequest.fromXContent(createParser(JsonXContent.jsonXContent, - queryWithNoTrackTotalHits)); - request.setRequest(searchRequest); - SearchTemplateResponse searchResponse1 = client().execute(SearchTemplateAction.INSTANCE, request).get(); - assertThat(searchResponse1.getResponse().getHits().getTotalHits().value, equalTo(10000L)); - - // When the request parameter `rest_total_hits_as_int` is set to true, `trackTotalHits` will also be set to true, - // we should test that we can get an accurate hits count. - SearchSourceBuilder builder = new SearchSourceBuilder(); - searchRequest.source(builder.trackTotalHits(true)); - request.setRequest(searchRequest); - SearchTemplateResponse searchResponse2 = client().execute(SearchTemplateAction.INSTANCE, request).get(); - assertThat(searchResponse2.getResponse().getHits().getTotalHits().value, equalTo(10001L)); - - // When `rest_total_hits_as_int` is set to true, `trackTotalHits` should not be set to a not accurate number - String queryWithInvalidTrackTotalHits = - "{" + " \"source\" : \"{ \\\"track_total_hits\\\": \\\"{{trackTotalHits}}\\\", \\\"query\\\":{\\\"match_all\\\":{}}}\"," - + " \"params\":{" - + " \"trackTotalHits\": 100" - + " }" - + "}"; - SearchTemplateRequest requestWithInvalidTrackTotalHits = SearchTemplateRequest.fromXContent(createParser(JsonXContent.jsonXContent, - queryWithInvalidTrackTotalHits)); - requestWithInvalidTrackTotalHits.setRequest(searchRequest); - Exception e = expectThrows(Exception.class, - () -> client().execute(SearchTemplateAction.INSTANCE, requestWithInvalidTrackTotalHits).get()); - assertThat(e.getMessage(), containsString( - "[rest_total_hits_as_int] cannot be used if the tracking of total hits is not accurate")); - - // When `rest_total_hits_as_int` is not set but `trackTotalHits` is set to true, we can also get an accurate hits count - String queryWithValidTrackTotalHits = - "{" + " \"source\" : \"{ \\\"track_total_hits\\\": \\\"{{trackTotalHits}}\\\", \\\"query\\\":{\\\"match_all\\\":{}}}\"," - + " \"params\":{" - + " \"trackTotalHits\": true" - + " }" - + "}"; - SearchTemplateRequest requestWithValidTrackTotalHits = SearchTemplateRequest.fromXContent(createParser(JsonXContent.jsonXContent, - queryWithValidTrackTotalHits)); - SearchRequest newSearchRequest = new SearchRequest(); - newSearchRequest.indices("test"); - requestWithValidTrackTotalHits.setRequest(newSearchRequest); - SearchTemplateResponse searchResponse3 = client().execute(SearchTemplateAction.INSTANCE, requestWithValidTrackTotalHits).get(); - assertThat(searchResponse3.getResponse().getHits().getTotalHits().value, equalTo(10001L)); - } - /** * Test that template can contain conditional clause. In this case it is at * the beginning of the string. diff --git a/modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/30_search_template.yml b/modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/30_search_template.yml index f4d4f3a97e5e0..a9d3c2da68617 100644 --- a/modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/30_search_template.yml +++ b/modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/30_search_template.yml @@ -40,6 +40,19 @@ - match: { hits.total: 2 } +--- +"Test with invalid track_total_hits": + + - do: + catch: bad_request + search_template: + rest_total_hits_as_int: true + body: { "source" : { "query": { "match_{{template}}": {} }, "track_total_hits": "{{trackTotalHits}}" }, "params" : { "template" : "all", "trackTotalHits" : 1 } } + + - match: { status: 400 } + - match: { error.type: illegal_argument_exception } + - match: { error.reason: "[rest_total_hits_as_int] cannot be used if the tracking of total hits is not accurate, got 1" } + --- "Missing template search request": diff --git a/modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/50_multi_search_template.yml b/modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/50_multi_search_template.yml index 8f72583b61d7c..e92e10b9ad276 100644 --- a/modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/50_multi_search_template.yml +++ b/modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/50_multi_search_template.yml @@ -94,6 +94,11 @@ setup: - source: '{"query": {"{{query_type}}": {} }' # Unknown query type params: query_type: "unknown" + # Search 4 has an unsupported track_total_hits + - index: index_* + - source: '{"query": {"match_all": {} }, "track_total_hits": "{{trackTotalHits}}" }' + params: + trackTotalHits: 1 - match: { responses.0.hits.total: 2 } - match: { responses.1.error.root_cause.0.type: json_e_o_f_exception } @@ -101,6 +106,9 @@ setup: - match: { responses.2.hits.total: 1 } - match: { responses.3.error.root_cause.0.type: parsing_exception } - match: { responses.3.error.root_cause.0.reason: "/unknown.query.\\[unknown\\]/" } + - match: { responses.4.error.root_cause.0.type: illegal_argument_exception } + - match: { responses.4.error.root_cause.0.reason: "[rest_total_hits_as_int] cannot be used if the tracking of total hits is not accurate, got 1" } + --- "Multi-search template with invalid request":