diff --git a/core/src/main/java/org/elasticsearch/action/search/ClearScrollRequest.java b/core/src/main/java/org/elasticsearch/action/search/ClearScrollRequest.java index 4f0643929cdae..f128332f8883d 100644 --- a/core/src/main/java/org/elasticsearch/action/search/ClearScrollRequest.java +++ b/core/src/main/java/org/elasticsearch/action/search/ClearScrollRequest.java @@ -37,6 +37,14 @@ public class ClearScrollRequest extends ActionRequest { private List scrollIds; + public ClearScrollRequest() { + + } + + public ClearScrollRequest(List scrollIds) { + this.scrollIds = scrollIds; + } + public List getScrollIds() { return scrollIds; } diff --git a/core/src/main/java/org/elasticsearch/rest/action/search/RestClearScrollAction.java b/core/src/main/java/org/elasticsearch/rest/action/search/RestClearScrollAction.java index bc49bffe44740..bfc5ab1e12769 100644 --- a/core/src/main/java/org/elasticsearch/rest/action/search/RestClearScrollAction.java +++ b/core/src/main/java/org/elasticsearch/rest/action/search/RestClearScrollAction.java @@ -20,7 +20,6 @@ package org.elasticsearch.rest.action.search; import org.elasticsearch.action.search.ClearScrollRequest; -import org.elasticsearch.action.search.ClearScrollResponse; import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesReference; @@ -37,47 +36,61 @@ import java.io.IOException; import java.util.Arrays; +import java.util.Collections; +import java.util.List; import static org.elasticsearch.rest.RestRequest.Method.DELETE; +import static org.elasticsearch.rest.RestRequest.Method.POST; public class RestClearScrollAction extends BaseRestHandler { @Inject public RestClearScrollAction(Settings settings, RestController controller) { super(settings); - + //the DELETE endpoints are both deprecated controller.registerHandler(DELETE, "/_search/scroll", this); controller.registerHandler(DELETE, "/_search/scroll/{scroll_id}", this); + controller.registerHandler(POST, "/_search/clear_scroll", this); + //the POST endpoint that accepts scroll_id in the url is deprecated too + controller.registerHandler(POST, "/_search/clear_scroll/{scroll_id}", this); } @Override public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException { - String scrollIds = request.param("scroll_id"); - ClearScrollRequest clearRequest = new ClearScrollRequest(); - clearRequest.setScrollIds(Arrays.asList(splitScrollIds(scrollIds))); + if (request.method() == DELETE) { + deprecationLogger.deprecated("Deprecated [DELETE /_search/scroll] endpoint used, expected [POST /_search/scroll] instead"); + } + if (request.method() == POST && request.hasParam("scroll_id")) { + deprecationLogger.deprecated("Deprecated [POST /_search/clear_scroll/{scroll_id}] endpoint used, " + + "expected [POST /_search/clear_scroll] instead. The scroll_id must be provided as part of the request body"); + } + final ClearScrollRequest clearRequest; if (RestActions.hasBodyContent(request)) { + //consume the scroll_id param, it will get ignored though (bw comp) + request.param("scroll_id"); XContentType type = RestActions.guessBodyContentType(request); - if (type == null) { - scrollIds = RestActions.getRestContent(request).utf8ToString(); - clearRequest.setScrollIds(Arrays.asList(splitScrollIds(scrollIds))); - } else { - // NOTE: if rest request with xcontent body has request parameters, these parameters does not override xcontent value - clearRequest.setScrollIds(null); - buildFromContent(RestActions.getRestContent(request), clearRequest); - } + if (type == null) { + String scrollIds = RestActions.getRestContent(request).utf8ToString(); + clearRequest = new ClearScrollRequest(splitScrollIds(scrollIds)); + } else { + clearRequest = parseClearScrollRequest(RestActions.getRestContent(request)); + } + } else { + clearRequest = new ClearScrollRequest(splitScrollIds(request.param("scroll_id"))); } - return channel -> client.clearScroll(clearRequest, new RestStatusToXContentListener(channel)); + return channel -> client.clearScroll(clearRequest, new RestStatusToXContentListener<>(channel)); } - public static String[] splitScrollIds(String scrollIds) { + private static List splitScrollIds(String scrollIds) { if (scrollIds == null) { - return Strings.EMPTY_ARRAY; + return Collections.emptyList(); } - return Strings.splitStringByCommaToArray(scrollIds); + return Arrays.asList(Strings.splitStringByCommaToArray(scrollIds)); } - public static void buildFromContent(BytesReference content, ClearScrollRequest clearScrollRequest) { + public static ClearScrollRequest parseClearScrollRequest(BytesReference content) { + ClearScrollRequest clearScrollRequest = new ClearScrollRequest(); try (XContentParser parser = XContentHelper.createParser(content)) { if (parser.nextToken() != XContentParser.Token.START_OBJECT) { throw new IllegalArgumentException("Malformed content, must start with an object"); @@ -103,6 +116,6 @@ public static void buildFromContent(BytesReference content, ClearScrollRequest c } catch (IOException e) { throw new IllegalArgumentException("Failed to parse request body", e); } + return clearScrollRequest; } - } diff --git a/core/src/test/java/org/elasticsearch/search/scroll/SearchScrollIT.java b/core/src/test/java/org/elasticsearch/search/scroll/SearchScrollIT.java index 275bc1c0c81b6..c908bcd3212b7 100644 --- a/core/src/test/java/org/elasticsearch/search/scroll/SearchScrollIT.java +++ b/core/src/test/java/org/elasticsearch/search/scroll/SearchScrollIT.java @@ -545,16 +545,13 @@ public void testParseClearScrollRequest() throws Exception { BytesReference content = XContentFactory.jsonBuilder().startObject() .array("scroll_id", "value_1", "value_2") .endObject().bytes(); - ClearScrollRequest clearScrollRequest = new ClearScrollRequest(); - RestClearScrollAction.buildFromContent(content, clearScrollRequest); + ClearScrollRequest clearScrollRequest = RestClearScrollAction.parseClearScrollRequest(content); assertThat(clearScrollRequest.scrollIds(), contains("value_1", "value_2")); } public void testParseClearScrollRequestWithInvalidJsonThrowsException() throws Exception { - ClearScrollRequest clearScrollRequest = new ClearScrollRequest(); - try { - RestClearScrollAction.buildFromContent(new BytesArray("{invalid_json}"), clearScrollRequest); + RestClearScrollAction.parseClearScrollRequest(new BytesArray("{invalid_json}")); fail("expected parseContent failure"); } catch (Exception e) { assertThat(e, instanceOf(IllegalArgumentException.class)); @@ -567,10 +564,8 @@ public void testParseClearScrollRequestWithUnknownParamThrowsException() throws .array("scroll_id", "value_1", "value_2") .field("unknown", "keyword") .endObject().bytes(); - ClearScrollRequest clearScrollRequest = new ClearScrollRequest(); - try { - RestClearScrollAction.buildFromContent(invalidContent, clearScrollRequest); + RestClearScrollAction.parseClearScrollRequest(invalidContent); fail("expected parseContent failure"); } catch (Exception e) { assertThat(e, instanceOf(IllegalArgumentException.class)); diff --git a/docs/reference/search/request/scroll.asciidoc b/docs/reference/search/request/scroll.asciidoc index 82a27881720fa..c29413e6e26d0 100644 --- a/docs/reference/search/request/scroll.asciidoc +++ b/docs/reference/search/request/scroll.asciidoc @@ -134,27 +134,15 @@ GET /_nodes/stats/indices/search ==== Clear scroll API -Search context are automatically removed when the `scroll` timeout has been +Search contexts are automatically removed when the `scroll` timeout has been exceeded. However keeping scrolls open has a cost, as discussed in the <> so scrolls should be explicitly -cleared as soon as the scroll is not being used anymore using the -`clear-scroll` API: +cleared as soon as they are not being used anymore using the `clear-scroll` +API. Multiple scroll IDs can be passed as array: [source,js] --------------------------------------- -DELETE /_search/scroll -{ - "scroll_id" : ["DXF1ZXJ5QW5kRmV0Y2gBAAAAAAAAAD4WYm9laVYtZndUQlNsdDcwakFMNjU1QQ=="] -} ---------------------------------------- -// CONSOLE -// TEST[catch:missing] - -Multiple scroll IDs can be passed as array: - -[source,js] ---------------------------------------- -DELETE /_search/scroll +POST /_search/clear_scroll { "scroll_id" : [ "DXF1ZXJ5QW5kRmV0Y2gBAAAAAAAAAD4WYm9laVYtZndUQlNsdDcwakFMNjU1QQ==", @@ -165,23 +153,20 @@ DELETE /_search/scroll // CONSOLE // TEST[catch:missing] -All search contexts can be cleared with the `_all` parameter: +deprecated[5.2.0,Providing the scroll ID as part of the URL or as a query string parameter has been deprecated in favour of specifying it in the request body. The `DELETE _search/scroll` endpoints are deprecated in favour of `POST _search/clear_scroll`] -[source,js] ---------------------------------------- -DELETE /_search/scroll/_all ---------------------------------------- -// CONSOLE - -The `scroll_id` can also be passed as a query string parameter or in the request body. -Multiple scroll IDs can be passed as comma separated values: +All search contexts can be cleared with the special `_all` scroll id: [source,js] --------------------------------------- -DELETE /_search/scroll/DXF1ZXJ5QW5kRmV0Y2gBAAAAAAAAAD4WYm9laVYtZndUQlNsdDcwakFMNjU1QQ==,DnF1ZXJ5VGhlbkZldGNoBQAAAAAAAAABFmtSWWRRWUJrU2o2ZExpSGJCVmQxYUEAAAAAAAAAAxZrUllkUVlCa1NqNmRMaUhiQlZkMWFBAAAAAAAAAAIWa1JZZFFZQmtTajZkTGlIYkJWZDFhQQAAAAAAAAAFFmtSWWRRWUJrU2o2ZExpSGJCVmQxYUEAAAAAAAAABBZrUllkUVlCa1NqNmRMaUhiQlZkMWFB +POST /_search/clear_scroll +{ + "scroll_id" : [ + "_all" + ] +} --------------------------------------- // CONSOLE -// TEST[catch:missing] [[sliced-scroll]] ==== Sliced Scroll diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/clear_scroll.json b/rest-api-spec/src/main/resources/rest-api-spec/api/clear_scroll.json index ecdfba142538d..c0d6c32aefd81 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/clear_scroll.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/clear_scroll.json @@ -1,10 +1,10 @@ { "clear_scroll": { "documentation": "https://www.elastic.co/guide/en/elasticsearch/reference/5.x/search-request-scroll.html", - "methods": ["DELETE"], + "methods": ["POST"], "url": { - "path": "/_search/scroll/{scroll_id}", - "paths": ["/_search/scroll/{scroll_id}", "/_search/scroll"], + "path": "/_search/clear_scroll/{scroll_id}", + "paths": ["/_search/clear_scroll/{scroll_id}", "/_search/clear_scroll"], "parts": { "scroll_id": { "type" : "list", @@ -14,7 +14,7 @@ "params": {} }, "body": { - "description": "A comma-separated list of scroll IDs to clear if none was specified via the scroll_id parameter" + "description": "A comma-separated list of scroll IDs to clear, has precedence over the scroll_id parameter" } } } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/delete_scroll.json b/rest-api-spec/src/main/resources/rest-api-spec/api/delete_scroll.json new file mode 100644 index 0000000000000..8830bc58da2f4 --- /dev/null +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/delete_scroll.json @@ -0,0 +1,20 @@ +{ + "delete_scroll": { + "documentation": "https://www.elastic.co/guide/en/elasticsearch/reference/5.x/search-request-scroll.html", + "methods": ["DELETE"], + "url": { + "path": "/_search/scroll/{scroll_id}", + "paths": ["/_search/scroll/{scroll_id}", "/_search/scroll"], + "parts": { + "scroll_id": { + "type" : "list", + "description" : "A comma-separated list of scroll IDs to clear" + } + }, + "params": {} + }, + "body": { + "description": "A comma-separated list of scroll IDs to clear, has precedence over the scroll_id parameter" + } + } +} diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/scroll/10_basic.yaml b/rest-api-spec/src/main/resources/rest-api-spec/test/scroll/10_basic.yaml index 4013f315d4633..cf625ac38aaca 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/scroll/10_basic.yaml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/scroll/10_basic.yaml @@ -63,7 +63,7 @@ - do: clear_scroll: - scroll_id: $scroll_id + body: { "scroll_id": [ "$scroll_id" ]} --- "Body params override query string": @@ -123,5 +123,5 @@ - do: clear_scroll: - scroll_id: $scroll_id + body: { "scroll_id": [ "$scroll_id" ]} diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/scroll/11_clear.yaml b/rest-api-spec/src/main/resources/rest-api-spec/test/scroll/11_clear.yaml index aa4885825d2ad..224cbc90bd35c 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/scroll/11_clear.yaml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/scroll/11_clear.yaml @@ -25,7 +25,7 @@ - do: clear_scroll: - scroll_id: $scroll_id1 + body: { "scroll_id": [ "$scroll_id1" ]} - do: catch: missing @@ -33,46 +33,11 @@ scroll_id: $scroll_id1 - do: - catch: missing - clear_scroll: - scroll_id: $scroll_id1 - ---- -"Body params override query string": - - do: - indices.create: - index: test_scroll - - do: - index: - index: test_scroll - type: test - id: 42 - body: { foo: bar } - - - do: - indices.refresh: {} - - - do: - search: - index: test_scroll - scroll: 1m - body: - query: - match_all: {} - - - set: {_scroll_id: scroll_id1} - - - do: + catch: missing clear_scroll: - scroll_id: "invalid_scroll_id" body: { "scroll_id": [ "$scroll_id1" ]} - do: catch: missing - scroll: - scroll_id: $scroll_id1 - - - do: - catch: missing - clear_scroll: - scroll_id: $scroll_id1 + clear_scroll: + body: { "scroll_id": [ "$scroll_id1" ]} diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/scroll/12_slices.yaml b/rest-api-spec/src/main/resources/rest-api-spec/test/scroll/12_slices.yaml index 1695bdb2352e0..d9c830419aefd 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/scroll/12_slices.yaml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/scroll/12_slices.yaml @@ -30,8 +30,8 @@ - set: {_scroll_id: scroll_id} - do: - clear_scroll: - scroll_id: $scroll_id + clear_scroll: + body: { "scroll_id": [ "$scroll_id" ]} - do: catch: /query_phase_execution_exception.*The number of slices.*index.max_slices_per_scroll/ @@ -67,6 +67,6 @@ - set: {_scroll_id: scroll_id} - do: - clear_scroll: - scroll_id: $scroll_id + clear_scroll: + body: { "scroll_id": [ "$scroll_id" ]} diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/scroll/13_clear_scroll_deprecated_endpoints.yaml b/rest-api-spec/src/main/resources/rest-api-spec/test/scroll/13_clear_scroll_deprecated_endpoints.yaml new file mode 100644 index 0000000000000..e6958c98731f4 --- /dev/null +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/scroll/13_clear_scroll_deprecated_endpoints.yaml @@ -0,0 +1,171 @@ +--- +setup: + - do: + indices.create: + index: test_scroll + - do: + index: + index: test_scroll + type: test + id: 42 + body: { foo: bar } + + - do: + indices.refresh: {} + +--- +"Delete scroll with request body": + + - skip: + features: "warnings" + version: " - 5.0.0" + reason: response headers were introduced in 5.0.0 + + - do: + search: + index: test_scroll + scroll: 1m + body: + query: + match_all: {} + + - set: {_scroll_id: scroll_id1} + + - do: + warnings: + - Deprecated [DELETE /_search/scroll] endpoint used, expected [POST /_search/scroll] instead + delete_scroll: + body: { "scroll_id": [ "$scroll_id1" ]} + + - do: + catch: missing + scroll: + scroll_id: $scroll_id1 + +--- +"Delete scroll without request body": + + - skip: + features: "warnings" + version: " - 5.0.0" + reason: response headers were introduced in 5.0.0 + + - do: + search: + index: test_scroll + scroll: 1m + body: + query: + match_all: {} + + - set: {_scroll_id: scroll_id1} + + - do: + warnings: + - Deprecated [DELETE /_search/scroll] endpoint used, expected [POST /_search/scroll] instead + delete_scroll: + "scroll_id": "$scroll_id1" + + - do: + catch: missing + scroll: + scroll_id: $scroll_id1 + +--- +"Delete scroll body params override query string": + + - skip: + features: "warnings" + version: " - 5.0.0" + reason: response headers were introduced in 5.0.0 + + - do: + search: + index: test_scroll + scroll: 1m + body: + query: + match_all: {} + + - set: {_scroll_id: scroll_id1} + + - do: + warnings: + - Deprecated [DELETE /_search/scroll] endpoint used, expected [POST /_search/scroll] instead + delete_scroll: + scroll_id: "invalid_scroll_id" + body: { "scroll_id": [ "$scroll_id1" ]} + + - do: + catch: missing + scroll: + scroll_id: $scroll_id1 + + - do: + catch: missing + clear_scroll: + body: { "scroll_id": [ "$scroll_id1" ]} + +--- +"Clear scroll without request body": + + - skip: + features: "warnings" + version: " - 5.0.0" + reason: response headers were introduced in 5.0.0 + + - do: + search: + index: test_scroll + scroll: 1m + body: + query: + match_all: {} + + - set: {_scroll_id: scroll_id1} + + - do: + warnings: + - Deprecated [POST /_search/clear_scroll/{scroll_id}] endpoint used, expected [POST /_search/clear_scroll] instead. The scroll_id must be provided as part of the request body + clear_scroll: + scroll_id: $scroll_id1 + + - do: + catch: missing + scroll: + scroll_id: $scroll_id1 + +--- +"Clear scroll body params override query string": + + - skip: + features: "warnings" + version: " - 5.0.0" + reason: response headers were introduced in 5.0.0 + + - do: + search: + index: test_scroll + scroll: 1m + body: + query: + match_all: {} + + - set: {_scroll_id: scroll_id1} + + - do: + warnings: + - Deprecated [POST /_search/clear_scroll/{scroll_id}] endpoint used, expected [POST /_search/clear_scroll] instead. The scroll_id must be provided as part of the request body + clear_scroll: + scroll_id: "invalid_scroll_id" + body: { "scroll_id": [ "$scroll_id1" ]} + + - do: + catch: missing + scroll: + scroll_id: $scroll_id1 + + - do: + catch: missing + clear_scroll: + body: { "scroll_id": [ "$scroll_id1" ]}