From 9e4105ec372c6411aef0efc9e7c7cdd7d66df1b7 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Thu, 5 Nov 2020 14:46:21 +0100 Subject: [PATCH] Validate PIT on _msearch (#63167) This change ensures that we validate point in times provided by individual search requests in _msearch. Relates #63132 --- .../action/search/RestMultiSearchAction.java | 15 ++++-- .../search/MultiSearchRequestTests.java | 10 ++-- .../10_data_stream_resolvability.yml | 12 ++--- .../test/search/point_in_time.yml | 53 +++++++++++++++---- 4 files changed, 64 insertions(+), 26 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/rest/action/search/RestMultiSearchAction.java b/server/src/main/java/org/elasticsearch/rest/action/search/RestMultiSearchAction.java index abddc9ba491..4cea7a392c2 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/search/RestMultiSearchAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/search/RestMultiSearchAction.java @@ -28,6 +28,7 @@ import org.elasticsearch.common.CheckedBiConsumer; import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.collect.Tuple; +import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContent; @@ -90,8 +91,7 @@ public class RestMultiSearchAction extends BaseRestHandler { @Override public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException { - MultiSearchRequest multiSearchRequest = parseRequest(request, allowExplicitIndex); - + final MultiSearchRequest multiSearchRequest = parseRequest(request, client.getNamedWriteableRegistry(), allowExplicitIndex); // Emit a single deprecation message if any search request contains types. for (SearchRequest searchRequest : multiSearchRequest.requests()) { if (searchRequest.types().length > 0) { @@ -108,7 +108,9 @@ public class RestMultiSearchAction extends BaseRestHandler { /** * Parses a {@link RestRequest} body and returns a {@link MultiSearchRequest} */ - public static MultiSearchRequest parseRequest(RestRequest restRequest, boolean allowExplicitIndex) throws IOException { + public static MultiSearchRequest parseRequest(RestRequest restRequest, + NamedWriteableRegistry namedWriteableRegistry, + boolean allowExplicitIndex) throws IOException { MultiSearchRequest multiRequest = new MultiSearchRequest(); IndicesOptions indicesOptions = IndicesOptions.fromRequest(restRequest, multiRequest.indicesOptions()); multiRequest.indicesOptions(indicesOptions); @@ -133,6 +135,13 @@ public class RestMultiSearchAction extends BaseRestHandler { parseMultiLineRequest(restRequest, multiRequest.indicesOptions(), allowExplicitIndex, (searchRequest, parser) -> { searchRequest.source(SearchSourceBuilder.fromXContent(parser, false)); RestSearchAction.checkRestTotalHits(restRequest, searchRequest); + if (searchRequest.pointInTimeBuilder() != null) { + RestSearchAction.preparePointInTime(searchRequest, restRequest, namedWriteableRegistry); + } else { + searchRequest.setCcsMinimizeRoundtrips( + restRequest.paramAsBoolean("ccs_minimize_roundtrips", searchRequest.isCcsMinimizeRoundtrips()) + ); + } multiRequest.add(searchRequest); }); List requests = multiRequest.requests(); diff --git a/server/src/test/java/org/elasticsearch/action/search/MultiSearchRequestTests.java b/server/src/test/java/org/elasticsearch/action/search/MultiSearchRequestTests.java index b21181e9447..1a97435df0e 100644 --- a/server/src/test/java/org/elasticsearch/action/search/MultiSearchRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/search/MultiSearchRequestTests.java @@ -106,7 +106,7 @@ public class MultiSearchRequestTests extends ESTestCase { FakeRestRequest restRequest = new FakeRestRequest.Builder(xContentRegistry()) .withContent(new BytesArray(requestContent), XContentType.JSON).build(); IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, - () -> RestMultiSearchAction.parseRequest(restRequest, true)); + () -> RestMultiSearchAction.parseRequest(restRequest, null, true)); assertEquals("key [unknown_key] is not supported in the metadata section", ex.getMessage()); } @@ -115,7 +115,7 @@ public class MultiSearchRequestTests extends ESTestCase { "{\"query\" : {\"match_all\" :{}}}\r\n"; FakeRestRequest restRequest = new FakeRestRequest.Builder(xContentRegistry()) .withContent(new BytesArray(requestContent), XContentType.JSON).build(); - MultiSearchRequest request = RestMultiSearchAction.parseRequest(restRequest, true); + MultiSearchRequest request = RestMultiSearchAction.parseRequest(restRequest, null, true); assertThat(request.requests().size(), equalTo(1)); assertThat(request.requests().get(0).indices()[0], equalTo("test")); assertThat(request.requests().get(0).indicesOptions(), @@ -130,7 +130,7 @@ public class MultiSearchRequestTests extends ESTestCase { .withContent(new BytesArray(requestContent), XContentType.JSON) .withParams(Collections.singletonMap("ignore_unavailable", "true")) .build(); - MultiSearchRequest request = RestMultiSearchAction.parseRequest(restRequest, true); + MultiSearchRequest request = RestMultiSearchAction.parseRequest(restRequest, null, true); assertThat(request.requests().size(), equalTo(1)); assertThat(request.requests().get(0).indices()[0], equalTo("test")); assertThat(request.requests().get(0).indicesOptions(), @@ -265,13 +265,13 @@ public class MultiSearchRequestTests extends ESTestCase { RestRequest restRequest = new FakeRestRequest.Builder(xContentRegistry()) .withContent(new BytesArray(mserchAction.getBytes(StandardCharsets.UTF_8)), XContentType.JSON).build(); IllegalArgumentException expectThrows = expectThrows(IllegalArgumentException.class, - () -> RestMultiSearchAction.parseRequest(restRequest, true)); + () -> RestMultiSearchAction.parseRequest(restRequest, null, true)); assertEquals("The msearch request must be terminated by a newline [\n]", expectThrows.getMessage()); String mserchActionWithNewLine = mserchAction + "\n"; RestRequest restRequestWithNewLine = new FakeRestRequest.Builder(xContentRegistry()) .withContent(new BytesArray(mserchActionWithNewLine.getBytes(StandardCharsets.UTF_8)), XContentType.JSON).build(); - MultiSearchRequest msearchRequest = RestMultiSearchAction.parseRequest(restRequestWithNewLine, true); + MultiSearchRequest msearchRequest = RestMultiSearchAction.parseRequest(restRequestWithNewLine, null, true); assertEquals(3, msearchRequest.requests().size()); } diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/data_stream/10_data_stream_resolvability.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/data_stream/10_data_stream_resolvability.yml index dd3ddccd049..617a86d7027 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/data_stream/10_data_stream_resolvability.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/data_stream/10_data_stream_resolvability.yml @@ -648,7 +648,6 @@ - do: search: - rest_total_hits_as_int: true body: size: 1 query: @@ -659,7 +658,7 @@ id: "$point_in_time_id" keep_alive: 1m - - match: {hits.total: 3 } + - match: {hits.total.value: 3 } - length: {hits.hits: 1 } - match: {hits.hits.0._index: .ds-simple-data-stream1-000001 } - match: {hits.hits.0._id: "123" } @@ -667,7 +666,6 @@ - do: search: - rest_total_hits_as_int: true body: size: 1 query: @@ -678,7 +676,7 @@ pit: id: "$point_in_time_id" - - match: {hits.total: 3} + - match: {hits.total.value: 3} - length: {hits.hits: 1 } - match: {hits.hits.0._index: .ds-simple-data-stream1-000001 } - match: {hits.hits.0._id: "5" } @@ -686,7 +684,6 @@ - do: search: - rest_total_hits_as_int: true body: size: 1 query: @@ -698,7 +695,7 @@ id: "$point_in_time_id" keep_alive: 1m - - match: {hits.total: 3} + - match: {hits.total.value: 3} - length: {hits.hits: 1 } - match: {hits.hits.0._index: .ds-simple-data-stream1-000001 } - match: {hits.hits.0._id: "1" } @@ -706,7 +703,6 @@ - do: search: - rest_total_hits_as_int: true body: size: 1 query: @@ -718,7 +714,7 @@ id: "$point_in_time_id" keep_alive: 1m - - match: {hits.total: 3} + - match: {hits.total.value: 3} - length: {hits.hits: 0 } - do: diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/search/point_in_time.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/search/point_in_time.yml index 6aad33f059d..66b2d18f120 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/search/point_in_time.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/search/point_in_time.yml @@ -47,7 +47,6 @@ setup: - do: search: - rest_total_hits_as_int: true body: size: 1 query: @@ -58,7 +57,7 @@ setup: id: "$point_in_time_id" keep_alive: 1m - - match: {hits.total: 3 } + - match: {hits.total.value: 3 } - length: {hits.hits: 1 } - match: {hits.hits.0._index: test } - match: {hits.hits.0._id: "172" } @@ -76,7 +75,6 @@ setup: # search with a point in time - do: search: - rest_total_hits_as_int: true body: size: 1 query: @@ -87,7 +85,7 @@ setup: pit: id: "$point_in_time_id" - - match: {hits.total: 3 } + - match: {hits.total.value: 3 } - length: {hits.hits: 1 } - match: {hits.hits.0._index: test } - match: {hits.hits.0._id: "42" } @@ -95,7 +93,6 @@ setup: - do: search: - rest_total_hits_as_int: true body: size: 1 query: @@ -106,7 +103,7 @@ setup: pit: id: "$point_in_time_id" - - match: {hits.total: 3} + - match: {hits.total.value: 3} - length: {hits.hits: 1 } - match: {hits.hits.0._index: test } - match: {hits.hits.0._id: "1" } @@ -114,7 +111,6 @@ setup: - do: search: - rest_total_hits_as_int: true body: size: 1 query: @@ -126,7 +122,7 @@ setup: id: "$point_in_time_id" keep_alive: 1m - - match: {hits.total: 3} + - match: {hits.total.value: 3} - length: {hits.hits: 0 } - do: @@ -147,7 +143,6 @@ setup: - do: search: - rest_total_hits_as_int: true body: size: 2 query: @@ -158,7 +153,7 @@ setup: id: "$point_in_time_id" keep_alive: 1m - - match: {hits.total: 4 } + - match: {hits.total.value: 4 } - length: {hits.hits: 2 } - match: {hits.hits.0._index: test } - match: {hits.hits.0._id: "172" } @@ -169,3 +164,41 @@ setup: close_point_in_time: body: id: "$point_in_time_id" + +--- +"msearch": + - skip: + version: " - 7.9.99" + reason: "point in time is introduced in 7.10" + - do: + open_point_in_time: + index: "t*" + keep_alive: 5m + - set: {id: point_in_time_id} + + - do: + msearch: + body: + - {} + - { query: { match: { _index: test }}, pit: { id: "$point_in_time_id" }} + - {} + - { query: { match_all: {}}, pit: { id: "$point_in_time_id" }} + + - match: { responses.0.hits.total.value: 3 } + - match: { responses.0.hits.total.relation: eq } + - match: { responses.1.hits.total.value: 4 } + - match: { responses.1.hits.total.relation: eq } + + - do: + catch: bad_request + msearch: + body: + - index: index + - { query: { match: { foo: bar }}, pit: { id: "$point_in_time_id" }} + - index: index2 + - { query: { match_all: {}}, pit: { id: "$point_in_time_id" }} + + - do: + close_point_in_time: + body: + id: "$point_in_time_id"