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 00000000000..46986438ee4 --- /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 8619143c83d..245093f3b38 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 class WatcherSearchTemplateRequest implements ToXContentObject { 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 b6111cb97e2..07fd3c77654 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 java.util.Map; 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 class WatcherSearchTemplateRequestTests extends ESTestCase { 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 @@ public class WatcherSearchTemplateRequestTests extends ESTestCase { 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 b4a05ef5a34..615fc8fbd08 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 @@ public abstract class AbstractWatcherIntegrationTestCase extends ESIntegTestCase 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 a037fba6c4d..2b7d5706d23 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 class BasicWatcherTests extends AbstractWatcherIntegrationTestCase { .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 class BasicWatcherTests extends AbstractWatcherIntegrationTestCase { .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 class BasicWatcherTests extends AbstractWatcherIntegrationTestCase { .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 class BasicWatcherTests extends AbstractWatcherIntegrationTestCase { .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 class BasicWatcherTests extends AbstractWatcherIntegrationTestCase { 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 class BasicWatcherTests extends AbstractWatcherIntegrationTestCase { 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 class BasicWatcherTests extends AbstractWatcherIntegrationTestCase { .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 class BasicWatcherTests extends AbstractWatcherIntegrationTestCase { 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 @@ public class BasicWatcherTests extends AbstractWatcherIntegrationTestCase { .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 805bda6f9d5..cb8fc9edb6f 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 class WatchAckTests extends AbstractWatcherIntegrationTestCase { .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 class WatchAckTests extends AbstractWatcherIntegrationTestCase { .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 class WatchAckTests extends AbstractWatcherIntegrationTestCase { .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();