Skip to content

Commit fb6a353

Browse files
javannajavanna
committed
SearchScrollRequest to implement ToXContentObject (#24906)
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)
1 parent 1f915b4 commit fb6a353

File tree

4 files changed

+130
-59
lines changed

4 files changed

+130
-59
lines changed

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

Lines changed: 39 additions & 5 deletions
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,11 +36,7 @@
3336

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

36-
/**
37-
*
38-
*/
39-
public class SearchScrollRequest extends ActionRequest {
40-
39+
public class SearchScrollRequest extends ActionRequest implements ToXContentObject {
4140
private String scrollId;
4241
private Scroll scroll;
4342

@@ -148,4 +147,39 @@ public String getDescription() {
148147
return "scrollId[" + scrollId + "], scroll[" + scroll + "]";
149148
}
150149

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

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

Lines changed: 2 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,6 @@
2323
import org.elasticsearch.client.node.NodeClient;
2424
import org.elasticsearch.common.bytes.BytesReference;
2525
import org.elasticsearch.common.settings.Settings;
26-
import org.elasticsearch.common.unit.TimeValue;
27-
import org.elasticsearch.common.xcontent.XContentParser;
2826
import org.elasticsearch.rest.BaseRestHandler;
2927
import org.elasticsearch.rest.RestController;
3028
import org.elasticsearch.rest.RestRequest;
@@ -56,7 +54,6 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
5654
if (scroll != null) {
5755
searchScrollRequest.scroll(new Scroll(parseTimeValue(scroll, null, "scroll")));
5856
}
59-
6057
request.withContentOrSourceParamParserOrNullLenient(xContentParser -> {
6158
if (xContentParser == null) {
6259
if (request.hasContent()) {
@@ -68,9 +65,9 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
6865
}
6966
}
7067
} else {
71-
// NOTE: if rest request with xcontent body has request parameters, these parameters override xcontent values
68+
// NOTE: if rest request with xcontent body has request parameters, values parsed from request body have the precedence
7269
try {
73-
buildFromContent(xContentParser, searchScrollRequest);
70+
searchScrollRequest.fromXContent(xContentParser);
7471
} catch (IOException e) {
7572
throw new IllegalArgumentException("Failed to parse request body", e);
7673
}
@@ -83,25 +80,4 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
8380
public boolean supportsPlainText() {
8481
return true;
8582
}
86-
87-
public static void buildFromContent(XContentParser parser, SearchScrollRequest searchScrollRequest) throws IOException {
88-
if (parser.nextToken() != XContentParser.Token.START_OBJECT) {
89-
throw new IllegalArgumentException("Malformed content, must start with an object");
90-
} else {
91-
XContentParser.Token token;
92-
String currentFieldName = null;
93-
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
94-
if (token == XContentParser.Token.FIELD_NAME) {
95-
currentFieldName = parser.currentName();
96-
} else if ("scroll_id".equals(currentFieldName) && token == XContentParser.Token.VALUE_STRING) {
97-
searchScrollRequest.scrollId(parser.text());
98-
} else if ("scroll".equals(currentFieldName) && token == XContentParser.Token.VALUE_STRING) {
99-
searchScrollRequest.scroll(new Scroll(TimeValue.parseTimeValue(parser.text(), null, "scroll")));
100-
} else {
101-
throw new IllegalArgumentException("Unknown parameter [" + currentFieldName
102-
+ "] in request body or parameter is of the wrong type[" + token + "] ");
103-
}
104-
}
105-
}
106-
}
10783
}

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

Lines changed: 64 additions & 0 deletions
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‎

Lines changed: 25 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -24,41 +24,30 @@
2424
import org.elasticsearch.client.node.NodeClient;
2525
import org.elasticsearch.common.bytes.BytesArray;
2626
import org.elasticsearch.common.settings.Settings;
27-
import org.elasticsearch.common.unit.TimeValue;
2827
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
29-
import org.elasticsearch.common.xcontent.XContentFactory;
30-
import org.elasticsearch.common.xcontent.XContentParser;
3128
import org.elasticsearch.common.xcontent.XContentType;
3229
import org.elasticsearch.rest.RestChannel;
3330
import org.elasticsearch.rest.RestController;
3431
import org.elasticsearch.rest.RestRequest;
3532
import org.elasticsearch.rest.action.search.RestSearchScrollAction;
3633
import org.elasticsearch.test.ESTestCase;
34+
import org.elasticsearch.test.rest.FakeRestChannel;
3735
import org.elasticsearch.test.rest.FakeRestRequest;
3836
import org.mockito.ArgumentCaptor;
3937

38+
import java.util.HashMap;
39+
import java.util.Map;
40+
4041
import java.util.Collections;
4142

42-
import static org.elasticsearch.mock.orig.Mockito.verify;
4343
import static org.hamcrest.Matchers.equalTo;
44-
import static org.hamcrest.Matchers.startsWith;
4544
import static org.mockito.Matchers.any;
45+
import static org.mockito.Matchers.anyObject;
46+
import static org.mockito.Mockito.doNothing;
4647
import static org.mockito.Mockito.mock;
48+
import static org.mockito.Mockito.verify;
4749

4850
public class RestSearchScrollActionTests extends ESTestCase {
49-
public void testParseSearchScrollRequest() throws Exception {
50-
XContentParser content = createParser(XContentFactory.jsonBuilder()
51-
.startObject()
52-
.field("scroll_id", "SCROLL_ID")
53-
.field("scroll", "1m")
54-
.endObject());
55-
56-
SearchScrollRequest searchScrollRequest = new SearchScrollRequest();
57-
RestSearchScrollAction.buildFromContent(content, searchScrollRequest);
58-
59-
assertThat(searchScrollRequest.scrollId(), equalTo("SCROLL_ID"));
60-
assertThat(searchScrollRequest.scroll().keepAlive(), equalTo(TimeValue.parseTimeValue("1m", null, "scroll")));
61-
}
6251

6352
public void testParseSearchScrollRequestWithInvalidJsonThrowsException() throws Exception {
6453
RestSearchScrollAction action = new RestSearchScrollAction(Settings.EMPTY, mock(RestController.class));
@@ -68,17 +57,25 @@ public void testParseSearchScrollRequestWithInvalidJsonThrowsException() throws
6857
assertThat(e.getMessage(), equalTo("Failed to parse request body"));
6958
}
7059

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

79-
Exception e = expectThrows(IllegalArgumentException.class,
80-
() -> RestSearchScrollAction.buildFromContent(invalidContent, searchScrollRequest));
81-
assertThat(e.getMessage(), startsWith("Unknown parameter [unknown]"));
74+
ArgumentCaptor<SearchScrollRequest> argument = ArgumentCaptor.forClass(SearchScrollRequest.class);
75+
verify(nodeClient).searchScroll(argument.capture(), anyObject());
76+
SearchScrollRequest searchScrollRequest = argument.getValue();
77+
assertEquals("BODY", searchScrollRequest.scrollId());
78+
assertEquals("1m", searchScrollRequest.scroll().keepAlive().getStringRep());
8279
}
8380

8481
public void testParseSearchScrollPlaintext() throws Exception {

0 commit comments

Comments
 (0)