Skip to content

Commit 6b227e0

Browse files
javannajavanna
committed
SearchScrollRequest to implement ToXContentObject
SearchScrollRequest can be created from a request body, but it doesn't support the opposite, meaning printing out its content to an XContentBuilder. This is useful to the high level REST client and allows for better testing of what we parse. Moved parsing method from RestSearchScrollAction to SearchScrollRequest so that fromXContent and toXContent sit close to each other. Added unit tests to verify that body parameters override query_string parameters when both present (there is already a yaml test for this but unit test is even better) Relates to elastic#3889
1 parent 74e031e commit 6b227e0

File tree

4 files changed

+133
-53
lines changed

4 files changed

+133
-53
lines changed

‎core/src/main/java/org/elasticsearch/action/search/SearchScrollRequest.java

+39-1
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@
2424
import org.elasticsearch.common.io.stream.StreamInput;
2525
import org.elasticsearch.common.io.stream.StreamOutput;
2626
import org.elasticsearch.common.unit.TimeValue;
27+
import org.elasticsearch.common.xcontent.ToXContentObject;
28+
import org.elasticsearch.common.xcontent.XContentBuilder;
29+
import org.elasticsearch.common.xcontent.XContentParser;
2730
import org.elasticsearch.search.Scroll;
2831
import org.elasticsearch.tasks.Task;
2932
import org.elasticsearch.tasks.TaskId;
@@ -33,7 +36,7 @@
3336

3437
import static org.elasticsearch.action.ValidateActions.addValidationError;
3538

36-
public class SearchScrollRequest extends ActionRequest {
39+
public class SearchScrollRequest extends ActionRequest implements ToXContentObject {
3740

3841
private String scrollId;
3942
private Scroll scroll;
@@ -145,4 +148,39 @@ public String getDescription() {
145148
return "scrollId[" + scrollId + "], scroll[" + scroll + "]";
146149
}
147150

151+
@Override
152+
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
153+
builder.startObject();
154+
builder.field("scroll_id", scrollId);
155+
if (scroll != null) {
156+
builder.field("scroll", scroll.keepAlive().getStringRep());
157+
}
158+
builder.endObject();
159+
return builder;
160+
}
161+
162+
/**
163+
* Parse a search scroll request from a request body provided through the REST layer.
164+
* Values that are already be set and are also found while parsing will be overridden.
165+
*/
166+
public void fromXContent(XContentParser parser) throws IOException {
167+
if (parser.nextToken() != XContentParser.Token.START_OBJECT) {
168+
throw new IllegalArgumentException("Malformed content, must start with an object");
169+
} else {
170+
XContentParser.Token token;
171+
String currentFieldName = null;
172+
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
173+
if (token == XContentParser.Token.FIELD_NAME) {
174+
currentFieldName = parser.currentName();
175+
} else if ("scroll_id".equals(currentFieldName) && token == XContentParser.Token.VALUE_STRING) {
176+
scrollId(parser.text());
177+
} else if ("scroll".equals(currentFieldName) && token == XContentParser.Token.VALUE_STRING) {
178+
scroll(new Scroll(TimeValue.parseTimeValue(parser.text(), null, "scroll")));
179+
} else {
180+
throw new IllegalArgumentException("Unknown parameter [" + currentFieldName
181+
+ "] in request body or parameter is of the wrong type[" + token + "] ");
182+
}
183+
}
184+
}
185+
}
148186
}

‎core/src/main/java/org/elasticsearch/rest/action/search/RestSearchScrollAction.java

+2-25
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@
2222
import org.elasticsearch.action.search.SearchScrollRequest;
2323
import org.elasticsearch.client.node.NodeClient;
2424
import org.elasticsearch.common.settings.Settings;
25-
import org.elasticsearch.common.unit.TimeValue;
26-
import org.elasticsearch.common.xcontent.XContentParser;
2725
import org.elasticsearch.rest.BaseRestHandler;
2826
import org.elasticsearch.rest.RestController;
2927
import org.elasticsearch.rest.RestRequest;
@@ -58,34 +56,13 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
5856

5957
request.withContentOrSourceParamParserOrNull(xContentParser -> {
6058
if (xContentParser != null) {
61-
// NOTE: if rest request with xcontent body has request parameters, these parameters override xcontent values
59+
// NOTE: if rest request with xcontent body has request parameters, values parsed from request body have the precedence
6260
try {
63-
buildFromContent(xContentParser, searchScrollRequest);
61+
searchScrollRequest.fromXContent(xContentParser);
6462
} catch (IOException e) {
6563
throw new IllegalArgumentException("Failed to parse request body", e);
6664
}
6765
}});
6866
return channel -> client.searchScroll(searchScrollRequest, new RestStatusToXContentListener<>(channel));
6967
}
70-
71-
public static void buildFromContent(XContentParser parser, SearchScrollRequest searchScrollRequest) throws IOException {
72-
if (parser.nextToken() != XContentParser.Token.START_OBJECT) {
73-
throw new IllegalArgumentException("Malformed content, must start with an object");
74-
} else {
75-
XContentParser.Token token;
76-
String currentFieldName = null;
77-
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
78-
if (token == XContentParser.Token.FIELD_NAME) {
79-
currentFieldName = parser.currentName();
80-
} else if ("scroll_id".equals(currentFieldName) && token == XContentParser.Token.VALUE_STRING) {
81-
searchScrollRequest.scrollId(parser.text());
82-
} else if ("scroll".equals(currentFieldName) && token == XContentParser.Token.VALUE_STRING) {
83-
searchScrollRequest.scroll(new Scroll(TimeValue.parseTimeValue(parser.text(), null, "scroll")));
84-
} else {
85-
throw new IllegalArgumentException("Unknown parameter [" + currentFieldName
86-
+ "] in request body or parameter is of the wrong type[" + token + "] ");
87-
}
88-
}
89-
}
90-
}
9168
}

‎core/src/test/java/org/elasticsearch/action/search/SearchScrollRequestTests.java

+64
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,25 @@
1919

2020
package org.elasticsearch.action.search;
2121

22+
import org.elasticsearch.common.bytes.BytesReference;
2223
import org.elasticsearch.common.io.stream.BytesStreamOutput;
2324
import org.elasticsearch.common.io.stream.StreamInput;
2425
import org.elasticsearch.common.unit.TimeValue;
26+
import org.elasticsearch.common.xcontent.ToXContent;
27+
import org.elasticsearch.common.xcontent.XContentBuilder;
28+
import org.elasticsearch.common.xcontent.XContentFactory;
29+
import org.elasticsearch.common.xcontent.XContentHelper;
30+
import org.elasticsearch.common.xcontent.XContentParser;
31+
import org.elasticsearch.common.xcontent.XContentType;
32+
import org.elasticsearch.common.xcontent.json.JsonXContent;
2533
import org.elasticsearch.search.internal.InternalScrollSearchRequest;
2634
import org.elasticsearch.test.ESTestCase;
2735

2836
import java.io.IOException;
2937

3038
import static org.elasticsearch.test.EqualsHashCodeTestUtils.checkEqualsAndHashCode;
39+
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertToXContentEquivalent;
40+
import static org.hamcrest.Matchers.startsWith;
3141

3242
public class SearchScrollRequestTests extends ESTestCase {
3343

@@ -60,6 +70,60 @@ public void testInternalScrollSearchRequestSerialization() throws IOException {
6070
}
6171
}
6272

73+
public void testFromXContent() throws Exception {
74+
SearchScrollRequest searchScrollRequest = new SearchScrollRequest();
75+
if (randomBoolean()) {
76+
//test that existing values get overridden
77+
searchScrollRequest = createSearchScrollRequest();
78+
}
79+
try (XContentParser parser = createParser(XContentFactory.jsonBuilder()
80+
.startObject()
81+
.field("scroll_id", "SCROLL_ID")
82+
.field("scroll", "1m")
83+
.endObject())) {
84+
searchScrollRequest.fromXContent(parser);
85+
}
86+
assertEquals("SCROLL_ID", searchScrollRequest.scrollId());
87+
assertEquals(TimeValue.parseTimeValue("1m", null, "scroll"), searchScrollRequest.scroll().keepAlive());
88+
}
89+
90+
public void testFromXContentWithUnknownParamThrowsException() throws Exception {
91+
SearchScrollRequest searchScrollRequest = new SearchScrollRequest();
92+
XContentParser invalidContent = createParser(XContentFactory.jsonBuilder()
93+
.startObject()
94+
.field("scroll_id", "value_2")
95+
.field("unknown", "keyword")
96+
.endObject());
97+
98+
Exception e = expectThrows(IllegalArgumentException.class,
99+
() -> searchScrollRequest.fromXContent(invalidContent));
100+
assertThat(e.getMessage(), startsWith("Unknown parameter [unknown]"));
101+
}
102+
103+
public void testToXContent() throws IOException {
104+
SearchScrollRequest searchScrollRequest = new SearchScrollRequest();
105+
searchScrollRequest.scrollId("SCROLL_ID");
106+
searchScrollRequest.scroll("1m");
107+
try (XContentBuilder builder = JsonXContent.contentBuilder()) {
108+
searchScrollRequest.toXContent(builder, ToXContent.EMPTY_PARAMS);
109+
assertEquals("{\"scroll_id\":\"SCROLL_ID\",\"scroll\":\"1m\"}", builder.string());
110+
}
111+
}
112+
113+
public void testToAndFromXContent() throws IOException {
114+
XContentType xContentType = randomFrom(XContentType.values());
115+
boolean humanReadable = randomBoolean();
116+
SearchScrollRequest originalRequest = createSearchScrollRequest();
117+
BytesReference originalBytes = toShuffledXContent(originalRequest, xContentType, ToXContent.EMPTY_PARAMS, humanReadable);
118+
SearchScrollRequest parsedRequest = new SearchScrollRequest();
119+
try (XContentParser parser = createParser(xContentType.xContent(), originalBytes)) {
120+
parsedRequest.fromXContent(parser);
121+
}
122+
assertEquals(originalRequest, parsedRequest);
123+
BytesReference parsedBytes = XContentHelper.toXContent(parsedRequest, xContentType, humanReadable);
124+
assertToXContentEquivalent(originalBytes, parsedBytes, xContentType);
125+
}
126+
63127
public void testEqualsAndHashcode() {
64128
checkEqualsAndHashCode(createSearchScrollRequest(), SearchScrollRequestTests::copyRequest, SearchScrollRequestTests::mutate);
65129
}

‎core/src/test/java/org/elasticsearch/search/scroll/RestSearchScrollActionTests.java

+28-27
Original file line numberDiff line numberDiff line change
@@ -20,36 +20,29 @@
2020
package org.elasticsearch.search.scroll;
2121

2222
import org.elasticsearch.action.search.SearchScrollRequest;
23+
import org.elasticsearch.client.node.NodeClient;
2324
import org.elasticsearch.common.bytes.BytesArray;
2425
import org.elasticsearch.common.settings.Settings;
25-
import org.elasticsearch.common.unit.TimeValue;
26-
import org.elasticsearch.common.xcontent.XContentFactory;
27-
import org.elasticsearch.common.xcontent.XContentParser;
2826
import org.elasticsearch.common.xcontent.XContentType;
2927
import org.elasticsearch.rest.RestController;
3028
import org.elasticsearch.rest.RestRequest;
3129
import org.elasticsearch.rest.action.search.RestSearchScrollAction;
3230
import org.elasticsearch.test.ESTestCase;
31+
import org.elasticsearch.test.rest.FakeRestChannel;
3332
import org.elasticsearch.test.rest.FakeRestRequest;
33+
import org.mockito.ArgumentCaptor;
34+
35+
import java.util.HashMap;
36+
import java.util.Map;
3437

3538
import static org.hamcrest.Matchers.equalTo;
36-
import static org.hamcrest.Matchers.startsWith;
39+
import static org.mockito.Matchers.any;
40+
import static org.mockito.Matchers.anyObject;
41+
import static org.mockito.Mockito.doNothing;
3742
import static org.mockito.Mockito.mock;
43+
import static org.mockito.Mockito.verify;
3844

3945
public class RestSearchScrollActionTests extends ESTestCase {
40-
public void testParseSearchScrollRequest() throws Exception {
41-
XContentParser content = createParser(XContentFactory.jsonBuilder()
42-
.startObject()
43-
.field("scroll_id", "SCROLL_ID")
44-
.field("scroll", "1m")
45-
.endObject());
46-
47-
SearchScrollRequest searchScrollRequest = new SearchScrollRequest();
48-
RestSearchScrollAction.buildFromContent(content, searchScrollRequest);
49-
50-
assertThat(searchScrollRequest.scrollId(), equalTo("SCROLL_ID"));
51-
assertThat(searchScrollRequest.scroll().keepAlive(), equalTo(TimeValue.parseTimeValue("1m", null, "scroll")));
52-
}
5346

5447
public void testParseSearchScrollRequestWithInvalidJsonThrowsException() throws Exception {
5548
RestSearchScrollAction action = new RestSearchScrollAction(Settings.EMPTY, mock(RestController.class));
@@ -59,16 +52,24 @@ public void testParseSearchScrollRequestWithInvalidJsonThrowsException() throws
5952
assertThat(e.getMessage(), equalTo("Failed to parse request body"));
6053
}
6154

62-
public void testParseSearchScrollRequestWithUnknownParamThrowsException() throws Exception {
63-
SearchScrollRequest searchScrollRequest = new SearchScrollRequest();
64-
XContentParser invalidContent = createParser(XContentFactory.jsonBuilder()
65-
.startObject()
66-
.field("scroll_id", "value_2")
67-
.field("unknown", "keyword")
68-
.endObject());
55+
public void testBodyParamsOverrideQueryStringParams() throws Exception {
56+
NodeClient nodeClient = mock(NodeClient.class);
57+
doNothing().when(nodeClient).searchScroll(any(), any());
58+
59+
RestSearchScrollAction action = new RestSearchScrollAction(Settings.EMPTY, mock(RestController.class));
60+
Map<String, String> params = new HashMap<>();
61+
params.put("scroll_id", "QUERY_STRING");
62+
params.put("scroll", "1000m");
63+
RestRequest request = new FakeRestRequest.Builder(xContentRegistry())
64+
.withParams(params)
65+
.withContent(new BytesArray("{\"scroll_id\":\"BODY\", \"scroll\":\"1m\"}"), XContentType.JSON).build();
66+
FakeRestChannel channel = new FakeRestChannel(request, false, 0);
67+
action.handleRequest(request, channel, nodeClient);
6968

70-
Exception e = expectThrows(IllegalArgumentException.class,
71-
() -> RestSearchScrollAction.buildFromContent(invalidContent, searchScrollRequest));
72-
assertThat(e.getMessage(), startsWith("Unknown parameter [unknown]"));
69+
ArgumentCaptor<SearchScrollRequest> argument = ArgumentCaptor.forClass(SearchScrollRequest.class);
70+
verify(nodeClient).searchScroll(argument.capture(), anyObject());
71+
SearchScrollRequest searchScrollRequest = argument.getValue();
72+
assertEquals("BODY", searchScrollRequest.scrollId());
73+
assertEquals("1m", searchScrollRequest.scroll().keepAlive().getStringRep());
7374
}
7475
}

0 commit comments

Comments
 (0)