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 23c5c3747fbf4..4770818867c84 100644 --- a/core/src/main/java/org/elasticsearch/action/search/ClearScrollRequest.java +++ b/core/src/main/java/org/elasticsearch/action/search/ClearScrollRequest.java @@ -23,6 +23,9 @@ import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.xcontent.ToXContentObject; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentParser; import java.io.IOException; import java.util.ArrayList; @@ -31,7 +34,7 @@ import static org.elasticsearch.action.ValidateActions.addValidationError; -public class ClearScrollRequest extends ActionRequest { +public class ClearScrollRequest extends ActionRequest implements ToXContentObject { private List scrollIds; @@ -83,4 +86,47 @@ public void writeTo(StreamOutput out) throws IOException { } } + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.startObject(); + builder.startArray("scroll_id"); + for (String scrollId : scrollIds) { + builder.value(scrollId); + } + builder.endArray(); + builder.endObject(); + return builder; + } + + public void fromXContent(XContentParser parser) throws IOException { + scrollIds = null; + if (parser.nextToken() != XContentParser.Token.START_OBJECT) { + throw new IllegalArgumentException("Malformed content, must start with an object"); + } else { + XContentParser.Token token; + String currentFieldName = null; + while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { + if (token == XContentParser.Token.FIELD_NAME) { + currentFieldName = parser.currentName(); + } else if ("scroll_id".equals(currentFieldName)){ + if (token == XContentParser.Token.START_ARRAY) { + while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) { + if (token.isValue() == false) { + throw new IllegalArgumentException("scroll_id array element should only contain scroll_id"); + } + addScrollId(parser.text()); + } + } else { + if (token.isValue() == false) { + throw new IllegalArgumentException("scroll_id element should only contain scroll_id"); + } + addScrollId(parser.text()); + } + } else { + throw new IllegalArgumentException("Unknown parameter [" + currentFieldName + + "] in request body or parameter is of the wrong type[" + token + "] "); + } + } + } + } } 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 c7281da23f137..80d833ed3118a 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 @@ -23,7 +23,6 @@ import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.common.Strings; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestRequest; @@ -49,10 +48,9 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC clearRequest.setScrollIds(Arrays.asList(splitScrollIds(scrollIds))); request.withContentOrSourceParamParserOrNull((xContentParser -> { if (xContentParser != null) { - // NOTE: if rest request with xcontent body has request parameters, these parameters does not override xcontent value - clearRequest.setScrollIds(null); + // NOTE: if rest request with xcontent body has request parameters, values parsed from request body have the precedence try { - buildFromContent(xContentParser, clearRequest); + clearRequest.fromXContent(xContentParser); } catch (IOException e) { throw new IllegalArgumentException("Failed to parse request body", e); } @@ -68,36 +66,4 @@ private static String[] splitScrollIds(String scrollIds) { } return Strings.splitStringByCommaToArray(scrollIds); } - - public static void buildFromContent(XContentParser parser, ClearScrollRequest clearScrollRequest) throws IOException { - if (parser.nextToken() != XContentParser.Token.START_OBJECT) { - throw new IllegalArgumentException("Malformed content, must start with an object"); - } else { - XContentParser.Token token; - String currentFieldName = null; - while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { - if (token == XContentParser.Token.FIELD_NAME) { - currentFieldName = parser.currentName(); - } else if ("scroll_id".equals(currentFieldName)){ - if (token == XContentParser.Token.START_ARRAY) { - while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) { - if (token.isValue() == false) { - throw new IllegalArgumentException("scroll_id array element should only contain scroll_id"); - } - clearScrollRequest.addScrollId(parser.text()); - } - } else { - if (token.isValue() == false) { - throw new IllegalArgumentException("scroll_id element should only contain scroll_id"); - } - clearScrollRequest.addScrollId(parser.text()); - } - } else { - throw new IllegalArgumentException("Unknown parameter [" + currentFieldName - + "] in request body or parameter is of the wrong type[" + token + "] "); - } - } - } - } - } diff --git a/core/src/test/java/org/elasticsearch/action/search/ClearScrollRequestTests.java b/core/src/test/java/org/elasticsearch/action/search/ClearScrollRequestTests.java new file mode 100644 index 0000000000000..6414e510069a0 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/action/search/ClearScrollRequestTests.java @@ -0,0 +1,112 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.action.search; + +import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.xcontent.ToXContent; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentHelper; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.common.xcontent.json.JsonXContent; +import org.elasticsearch.test.ESTestCase; + +import java.io.IOException; + +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertToXContentEquivalent; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.startsWith; + +public class ClearScrollRequestTests extends ESTestCase { + + public void testFromXContent() throws Exception { + ClearScrollRequest clearScrollRequest = new ClearScrollRequest(); + if (randomBoolean()) { + //test that existing values get overridden + clearScrollRequest = createClearScrollRequest(); + } + try (XContentParser parser = createParser(XContentFactory.jsonBuilder() + .startObject() + .array("scroll_id", "value_1", "value_2") + .endObject())) { + clearScrollRequest.fromXContent(parser); + } + assertThat(clearScrollRequest.scrollIds(), contains("value_1", "value_2")); + } + + public void testFromXContentWithoutArray() throws Exception { + ClearScrollRequest clearScrollRequest = new ClearScrollRequest(); + if (randomBoolean()) { + //test that existing values get overridden + clearScrollRequest = createClearScrollRequest(); + } + try (XContentParser parser = createParser(XContentFactory.jsonBuilder() + .startObject() + .field("scroll_id", "value_1") + .endObject())) { + clearScrollRequest.fromXContent(parser); + } + assertThat(clearScrollRequest.scrollIds(), contains("value_1")); + } + + public void testFromXContentWithUnknownParamThrowsException() throws Exception { + XContentParser invalidContent = createParser(XContentFactory.jsonBuilder() + .startObject() + .array("scroll_id", "value_1", "value_2") + .field("unknown", "keyword") + .endObject()); + ClearScrollRequest clearScrollRequest = new ClearScrollRequest(); + + Exception e = expectThrows(IllegalArgumentException.class, () -> clearScrollRequest.fromXContent(invalidContent)); + assertThat(e.getMessage(), startsWith("Unknown parameter [unknown]")); + } + + public void testToXContent() throws IOException { + ClearScrollRequest clearScrollRequest = new ClearScrollRequest(); + clearScrollRequest.addScrollId("SCROLL_ID"); + try (XContentBuilder builder = JsonXContent.contentBuilder()) { + clearScrollRequest.toXContent(builder, ToXContent.EMPTY_PARAMS); + assertEquals("{\"scroll_id\":[\"SCROLL_ID\"]}", builder.string()); + } + } + + public void testFromAndToXContent() throws IOException { + XContentType xContentType = randomFrom(XContentType.values()); + ClearScrollRequest originalRequest = createClearScrollRequest(); + BytesReference originalBytes = toShuffledXContent(originalRequest, xContentType, ToXContent.EMPTY_PARAMS, randomBoolean()); + ClearScrollRequest parsedRequest = new ClearScrollRequest(); + try (XContentParser parser = createParser(xContentType.xContent(), originalBytes)) { + parsedRequest.fromXContent(parser); + } + assertEquals(originalRequest.scrollIds(), parsedRequest.scrollIds()); + BytesReference parsedBytes = XContentHelper.toXContent(parsedRequest, xContentType, randomBoolean()); + assertToXContentEquivalent(originalBytes, parsedBytes, xContentType); + } + + public static ClearScrollRequest createClearScrollRequest() { + ClearScrollRequest clearScrollRequest = new ClearScrollRequest(); + int numScrolls = randomIntBetween(1, 10); + for (int i = 0; i < numScrolls; i++) { + clearScrollRequest.addScrollId(randomAlphaOfLengthBetween(3, 10)); + } + return clearScrollRequest; + } +} diff --git a/core/src/test/java/org/elasticsearch/search/scroll/RestClearScrollActionTests.java b/core/src/test/java/org/elasticsearch/search/scroll/RestClearScrollActionTests.java index ae8ad66ac8d94..905bddbcf161a 100644 --- a/core/src/test/java/org/elasticsearch/search/scroll/RestClearScrollActionTests.java +++ b/core/src/test/java/org/elasticsearch/search/scroll/RestClearScrollActionTests.java @@ -20,32 +20,29 @@ package org.elasticsearch.search.scroll; import org.elasticsearch.action.search.ClearScrollRequest; +import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.xcontent.XContentFactory; -import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.action.search.RestClearScrollAction; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.rest.FakeRestChannel; import org.elasticsearch.test.rest.FakeRestRequest; +import org.mockito.ArgumentCaptor; + +import java.util.Collections; +import java.util.List; -import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.startsWith; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyObject; +import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; public class RestClearScrollActionTests extends ESTestCase { - public void testParseClearScrollRequest() throws Exception { - XContentParser content = createParser(XContentFactory.jsonBuilder() - .startObject() - .array("scroll_id", "value_1", "value_2") - .endObject()); - ClearScrollRequest clearScrollRequest = new ClearScrollRequest(); - RestClearScrollAction.buildFromContent(content, clearScrollRequest); - assertThat(clearScrollRequest.scrollIds(), contains("value_1", "value_2")); - } public void testParseClearScrollRequestWithInvalidJsonThrowsException() throws Exception { RestClearScrollAction action = new RestClearScrollAction(Settings.EMPTY, mock(RestController.class)); @@ -55,17 +52,22 @@ public void testParseClearScrollRequestWithInvalidJsonThrowsException() throws E assertThat(e.getMessage(), equalTo("Failed to parse request body")); } - public void testParseClearScrollRequestWithUnknownParamThrowsException() throws Exception { - XContentParser invalidContent = createParser(XContentFactory.jsonBuilder() - .startObject() - .array("scroll_id", "value_1", "value_2") - .field("unknown", "keyword") - .endObject()); - ClearScrollRequest clearScrollRequest = new ClearScrollRequest(); + public void testBodyParamsOverrideQueryStringParams() throws Exception { + NodeClient nodeClient = mock(NodeClient.class); + doNothing().when(nodeClient).searchScroll(any(), any()); - Exception e = expectThrows(IllegalArgumentException.class, - () -> RestClearScrollAction.buildFromContent(invalidContent, clearScrollRequest)); - assertThat(e.getMessage(), startsWith("Unknown parameter [unknown]")); - } + RestClearScrollAction action = new RestClearScrollAction(Settings.EMPTY, mock(RestController.class)); + RestRequest request = new FakeRestRequest.Builder(xContentRegistry()) + .withParams(Collections.singletonMap("scroll_id", "QUERY_STRING")) + .withContent(new BytesArray("{\"scroll_id\": [\"BODY\"]}"), XContentType.JSON).build(); + FakeRestChannel channel = new FakeRestChannel(request, false, 0); + action.handleRequest(request, channel, nodeClient); + ArgumentCaptor argument = ArgumentCaptor.forClass(ClearScrollRequest.class); + verify(nodeClient).clearScroll(argument.capture(), anyObject()); + ClearScrollRequest clearScrollRequest = argument.getValue(); + List scrollIds = clearScrollRequest.getScrollIds(); + assertEquals(1, scrollIds.size()); + assertEquals("BODY", scrollIds.get(0)); + } }