From 00521a5b36fe2d91e9c669ff4f0ebd5a2b3f39f5 Mon Sep 17 00:00:00 2001 From: Alexander Reelsen Date: Tue, 18 Dec 2018 10:11:39 +0100 Subject: [PATCH] Watcher: Ensure all internal search requests count hits (#36697) In previous commits only the stored toXContent version of a search request was using the old format. However an executed search request was already disabling hit counts. In 7.0 hit counts will stay enabled by default to allow for proper migration. Closes #36177 --- .../put_watch/91_search_total_hits_as_int.yml | 115 ++++++++++++++++++ .../search/WatcherSearchTemplateRequest.java | 5 +- .../WatcherSearchTemplateRequestTests.java | 24 +++- .../AbstractWatcherIntegrationTestCase.java | 2 +- .../test/integration/BasicWatcherTests.java | 18 +-- .../test/integration/WatchAckTests.java | 6 +- 6 files changed, 152 insertions(+), 18 deletions(-) create mode 100644 x-pack/plugin/src/test/resources/rest-api-spec/test/watcher/put_watch/91_search_total_hits_as_int.yml diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/watcher/put_watch/91_search_total_hits_as_int.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/watcher/put_watch/91_search_total_hits_as_int.yml new file mode 100644 index 0000000000000..46986438ee4a4 --- /dev/null +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/watcher/put_watch/91_search_total_hits_as_int.yml @@ -0,0 +1,115 @@ +--- +setup: + - do: + cluster.health: + wait_for_status: yellow + + - do: + index: + index: my_test_index + type: doc + id: my_id + refresh: true + body: > + { + "key": "value" + } + +--- +"Test search input includes hits by default": + + - do: + xpack.watcher.execute_watch: + body: > + { + "watch" : { + "trigger": { + "schedule" : { "cron" : "0 0 0 1 * ? 2099" } + }, + "input": { + "search" : { + "request" : { + "indices" : [ "my_test_index" ], + "body" : { + "query": { + "match_all" : {} + } + } + } + } + }, + "condition": { + "compare": { + "ctx.payload.hits.total": { + "gt": 0 + } + } + }, + "actions": { + "logging" : { + "logging" : { + "text" : "Logging from a test" + } + } + } + } + } + + - match: { watch_record.result.condition.met: true } + + +--- +"Test search transform includes hits by default": + + - do: + xpack.watcher.execute_watch: + body: > + { + "watch" : { + "trigger": { + "schedule" : { "cron" : "0 0 0 1 * ? 2099" } + }, + "input": { + "simple": { + "foo": "bar" + } + }, + "transform" : { + "search" : { + "request" : { + "indices" : [ "my_test_index" ], + "body" : { + "query": { + "match_all" : {} + } + } + } + } + }, + "actions": { + "indexme" : { + "condition": { + "compare": { + "ctx.payload.hits.total": { + "gt": 0 + } + } + }, + "index" : { + "index" : "my_test_index", + "doc_type" : "doc", + "doc_id": "my-id" + } + } + } + } + } + + - do: + get: + index: my_test_index + type: doc + id: my_id + + - match: { _source.key: "value" } + diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/support/search/WatcherSearchTemplateRequest.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/support/search/WatcherSearchTemplateRequest.java index 8619143c83d38..245093f3b385c 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/support/search/WatcherSearchTemplateRequest.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/support/search/WatcherSearchTemplateRequest.java @@ -41,7 +41,7 @@ public class WatcherSearchTemplateRequest implements ToXContentObject { private final IndicesOptions indicesOptions; private final Script template; private final BytesReference searchSource; - private boolean restTotalHitsAsInt; + private boolean restTotalHitsAsInt = true; public WatcherSearchTemplateRequest(String[] indices, String[] types, SearchType searchType, IndicesOptions indicesOptions, BytesReference searchSource) { @@ -184,7 +184,8 @@ public static WatcherSearchTemplateRequest fromXContent(XContentParser parser, S IndicesOptions indicesOptions = DEFAULT_INDICES_OPTIONS; BytesReference searchSource = null; Script template = null; - boolean totalHitsAsInt = false; + // TODO this is to retain BWC compatibility in 7.0 and can be removed for 8.0 + boolean totalHitsAsInt = true; XContentParser.Token token; String currentFieldName = null; diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/support/search/WatcherSearchTemplateRequestTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/support/search/WatcherSearchTemplateRequestTests.java index b6111cb97e2b7..07fd3c7765485 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/support/search/WatcherSearchTemplateRequestTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/support/search/WatcherSearchTemplateRequestTests.java @@ -15,6 +15,7 @@ import static java.util.Collections.singletonMap; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; public class WatcherSearchTemplateRequestTests extends ESTestCase { @@ -28,7 +29,26 @@ public void testFromXContentWithTemplateCustomLang() throws IOException { assertTemplate(source, "custom-script", "painful", singletonMap("bar", "baz")); } - private void assertTemplate(String source, String expectedScript, String expectedLang, Map expectedParams) { + public void testDefaultHitCountsDefaults() throws IOException { + assertHitCount("{}", true); + } + + public void testDefaultHitCountsConfigured() throws IOException { + boolean hitCountsAsInt = randomBoolean(); + String source = "{ \"rest_total_hits_as_int\" : " + hitCountsAsInt + " }"; + assertHitCount(source, hitCountsAsInt); + } + + private void assertHitCount(String source, boolean expectedHitCountAsInt) throws IOException { + try (XContentParser parser = createParser(JsonXContent.jsonXContent, source)) { + parser.nextToken(); + WatcherSearchTemplateRequest request = WatcherSearchTemplateRequest.fromXContent(parser, SearchType.QUERY_THEN_FETCH); + assertThat(request.isRestTotalHitsAsint(), is(expectedHitCountAsInt)); + } + } + + private void assertTemplate(String source, String expectedScript, String expectedLang, Map expectedParams) + throws IOException { try (XContentParser parser = createParser(JsonXContent.jsonXContent, source)) { parser.nextToken(); WatcherSearchTemplateRequest result = WatcherSearchTemplateRequest.fromXContent(parser, randomFrom(SearchType.values())); @@ -36,8 +56,6 @@ private void assertTemplate(String source, String expectedScript, String expecte assertThat(result.getTemplate().getIdOrCode(), equalTo(expectedScript)); assertThat(result.getTemplate().getLang(), equalTo(expectedLang)); assertThat(result.getTemplate().getParams(), equalTo(expectedParams)); - } catch (IOException e) { - fail("Failed to parse watch search request: " + e.getMessage()); } } } diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/test/AbstractWatcherIntegrationTestCase.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/test/AbstractWatcherIntegrationTestCase.java index b4a05ef5a3439..615fc8fbd08f9 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/test/AbstractWatcherIntegrationTestCase.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/test/AbstractWatcherIntegrationTestCase.java @@ -340,7 +340,7 @@ protected void assertWatchWithMinimumPerformedActionsCount(final String watchNam assertThat("could not find executed watch record for watch " + watchName, searchResponse.getHits().getTotalHits().value, greaterThanOrEqualTo(minimumExpectedWatchActionsWithActionPerformed)); if (assertConditionMet) { - assertThat((Integer) XContentMapValues.extractValue("result.input.payload.hits.total.value", + assertThat((Integer) XContentMapValues.extractValue("result.input.payload.hits.total", searchResponse.getHits().getAt(0).getSourceAsMap()), greaterThanOrEqualTo(1)); } }); diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/test/integration/BasicWatcherTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/test/integration/BasicWatcherTests.java index a037fba6c4db8..2b7d5706d233e 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/test/integration/BasicWatcherTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/test/integration/BasicWatcherTests.java @@ -75,7 +75,7 @@ public void testIndexWatch() throws Exception { .setSource(watchBuilder() .trigger(schedule(interval(5, IntervalSchedule.Interval.Unit.SECONDS))) .input(searchInput(request)) - .condition(new CompareCondition("ctx.payload.hits.total.value", CompareCondition.Op.EQ, 1L)) + .condition(new CompareCondition("ctx.payload.hits.total", CompareCondition.Op.EQ, 1L)) .addAction("_logger", loggingAction("_logging") .setCategory("_category"))) .get(); @@ -95,7 +95,7 @@ public void testIndexWatchRegisterWatchBeforeTargetIndex() throws Exception { .setSource(watchBuilder() .trigger(schedule(interval(5, IntervalSchedule.Interval.Unit.SECONDS))) .input(searchInput(searchRequest)) - .condition(new CompareCondition("ctx.payload.hits.total.value", CompareCondition.Op.EQ, 1L))) + .condition(new CompareCondition("ctx.payload.hits.total", CompareCondition.Op.EQ, 1L))) .get(); timeWarp().trigger("_name"); // The watch's condition won't meet because there is no data that matches with the query @@ -119,7 +119,7 @@ public void testDeleteWatch() throws Exception { .setSource(watchBuilder() .trigger(schedule(cron("0/1 * * * * ? 2020"))) .input(searchInput(searchRequest)) - .condition(new CompareCondition("ctx.payload.hits.total.value", CompareCondition.Op.EQ, 1L))) + .condition(new CompareCondition("ctx.payload.hits.total", CompareCondition.Op.EQ, 1L))) .get(); assertThat(indexResponse.isCreated(), is(true)); DeleteWatchResponse deleteWatchResponse = watcherClient.prepareDeleteWatch("_name").get(); @@ -180,7 +180,7 @@ public void testModifyWatches() throws Exception { .addAction("_id", indexAction("idx", "action")); watcherClient().preparePutWatch("_name") - .setSource(source.condition(new CompareCondition("ctx.payload.hits.total.value", CompareCondition.Op.EQ, 1L))) + .setSource(source.condition(new CompareCondition("ctx.payload.hits.total", CompareCondition.Op.EQ, 1L))) .get(); timeWarp().clock().fastForwardSeconds(5); @@ -188,7 +188,7 @@ public void testModifyWatches() throws Exception { assertWatchWithMinimumPerformedActionsCount("_name", 0, false); watcherClient().preparePutWatch("_name") - .setSource(source.condition(new CompareCondition("ctx.payload.hits.total.value", CompareCondition.Op.EQ, 0L))) + .setSource(source.condition(new CompareCondition("ctx.payload.hits.total", CompareCondition.Op.EQ, 0L))) .get(); timeWarp().clock().fastForwardSeconds(5); @@ -199,7 +199,7 @@ public void testModifyWatches() throws Exception { watcherClient().preparePutWatch("_name") .setSource(source .trigger(schedule(Schedules.cron("0/1 * * * * ? 2020"))) - .condition(new CompareCondition("ctx.payload.hits.total.value", CompareCondition.Op.EQ, 0L))) + .condition(new CompareCondition("ctx.payload.hits.total", CompareCondition.Op.EQ, 0L))) .get(); timeWarp().clock().fastForwardSeconds(5); @@ -245,7 +245,7 @@ public void testInputFiltering() throws Exception { .setSource(watchBuilder() .trigger(schedule(interval(5, IntervalSchedule.Interval.Unit.SECONDS))) .input(searchInput(request).extractKeys("hits.total.value")) - .condition(new CompareCondition("ctx.payload.hits.total.value", CompareCondition.Op.EQ, 1L))) + .condition(new CompareCondition("ctx.payload.hits.total", CompareCondition.Op.EQ, 1L))) .get(); // in this watcher the condition will fail, because max_score isn't extracted, only total: watcherClient.preparePutWatch("_name2") @@ -265,7 +265,7 @@ public void testInputFiltering() throws Exception { SearchResponse searchResponse = searchWatchRecords(builder -> builder.setQuery(matchQuery("watch_id", "_name1"))); assertHitCount(searchResponse, 1); XContentSource source = xContentSource(searchResponse.getHits().getAt(0).getSourceRef()); - assertThat(source.getValue("result.input.payload.hits.total.value"), equalTo((Object) 1)); + assertThat(source.getValue("result.input.payload.hits.total"), equalTo((Object) 1)); } public void testPutWatchWithNegativeSchedule() throws Exception { @@ -349,7 +349,7 @@ private void testConditionSearch(WatcherSearchTemplateRequest request) throws Ex .setSource(watchBuilder() .trigger(schedule(interval("5s"))) .input(searchInput(request)) - .condition(new CompareCondition("ctx.payload.hits.total.value", CompareCondition.Op.GTE, 3L))) + .condition(new CompareCondition("ctx.payload.hits.total", CompareCondition.Op.GTE, 3L))) .get(); logger.info("created watch [{}] at [{}]", watchName, new DateTime(Clock.systemUTC().millis())); diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/test/integration/WatchAckTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/test/integration/WatchAckTests.java index 805bda6f9d593..cb8fc9edb6f14 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/test/integration/WatchAckTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/test/integration/WatchAckTests.java @@ -67,7 +67,7 @@ public void testAckSingleAction() throws Exception { .setSource(watchBuilder() .trigger(schedule(cron("0/5 * * * * ? *"))) .input(searchInput(templateRequest(searchSource(), "events"))) - .condition(new CompareCondition("ctx.payload.hits.total.value", CompareCondition.Op.GT, 0L)) + .condition(new CompareCondition("ctx.payload.hits.total", CompareCondition.Op.GT, 0L)) .transform(searchTransform(templateRequest(searchSource(), "events"))) .addAction("_a1", indexAction("actions1", "doc")) .addAction("_a2", indexAction("actions2", "doc")) @@ -127,7 +127,7 @@ public void testAckAllActions() throws Exception { .setSource(watchBuilder() .trigger(schedule(cron("0/5 * * * * ? *"))) .input(searchInput(templateRequest(searchSource(), "events"))) - .condition(new CompareCondition("ctx.payload.hits.total.value", CompareCondition.Op.GT, 0L)) + .condition(new CompareCondition("ctx.payload.hits.total", CompareCondition.Op.GT, 0L)) .transform(searchTransform(templateRequest(searchSource(), "events"))) .addAction("_a1", indexAction("actions1", "doc")) .addAction("_a2", indexAction("actions2", "doc")) @@ -195,7 +195,7 @@ public void testAckWithRestart() throws Exception { .setSource(watchBuilder() .trigger(schedule(cron("0/5 * * * * ? *"))) .input(searchInput(templateRequest(searchSource(), "events"))) - .condition(new CompareCondition("ctx.payload.hits.total.value", CompareCondition.Op.GT, 0L)) + .condition(new CompareCondition("ctx.payload.hits.total", CompareCondition.Op.GT, 0L)) .transform(searchTransform(templateRequest(searchSource(), "events"))) .addAction("_id", indexAction("actions", "action"))) .get();