Skip to content

Commit 776ed02

Browse files
jtibshiranijtibshirani
committed
Address code review feedback.
1 parent ae648c3 commit 776ed02

File tree

7 files changed

+224
-184
lines changed

7 files changed

+224
-184
lines changed

‎client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -519,8 +519,6 @@ static Request searchTemplate(SearchTemplateRequest searchTemplateRequest) throw
519519
request = new Request(HttpGet.METHOD_NAME, "_render/template");
520520
} else {
521521
SearchRequest searchRequest = searchTemplateRequest.getRequest();
522-
assert searchRequest != null : "When not simulating a template request, a search request must be present.";
523-
524522
String endpoint = endpoint(searchRequest.indices(), searchRequest.types(), "_search/template");
525523
request = new Request(HttpGet.METHOD_NAME, endpoint);
526524

‎client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/SearchDocumentationIT.java‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@
1919

2020
package org.elasticsearch.client.documentation;
2121

22-
import org.apache.http.entity.ContentType;
23-
import org.apache.http.nio.entity.NStringEntity;
2422
import org.elasticsearch.action.ActionListener;
2523
import org.elasticsearch.action.LatchedActionListener;
2624
import org.elasticsearch.action.admin.indices.create.CreateIndexRequest;
@@ -741,24 +739,26 @@ public void testSearchTemplateWithInlineScript() throws Exception {
741739
// tag::search-template-response
742740
SearchTemplateResponse response = client.searchTemplate(request);
743741
SearchResponse searchResponse = response.getResponse();
742+
// end::search-template-response
743+
744744
assertNotNull(searchResponse);
745745
assertTrue(searchResponse.getHits().totalHits > 0);
746-
// end::search-template-response
747746

748747
// tag::render-search-template-request
749748
request.setSimulate(true); // <1>
750749
// end::render-search-template-request
751750

752751
// tag::render-search-template-response
753752
SearchTemplateResponse renderResponse = client.searchTemplate(request);
754-
BytesReference source = renderResponse.getSource();
753+
BytesReference source = renderResponse.getSource(); // <1>
754+
// end::render-search-template-response
755+
755756
assertNotNull(source);
756757
assertEquals((
757758
"{" +
758759
" \"size\" : \"5\"," +
759760
" \"query\": { \"match\" : { \"title\" : \"elasticsearch\" } }" +
760761
"}").replaceAll("\\s+", ""), source.utf8ToString());
761-
// end::render-search-template-response
762762
}
763763

764764
public void testSearchTemplateWithStoredScript() throws Exception {
@@ -768,7 +768,7 @@ public void testSearchTemplateWithStoredScript() throws Exception {
768768

769769
// tag::register-script
770770
Request scriptRequest = new Request("POST", "_scripts/title_search");
771-
scriptRequest.setEntity(new NStringEntity(
771+
scriptRequest.setJsonEntity(
772772
"{" +
773773
" \"script\": {" +
774774
" \"lang\": \"mustache\"," +
@@ -777,10 +777,10 @@ public void testSearchTemplateWithStoredScript() throws Exception {
777777
" \"size\" : \"{{size}}\"" +
778778
" }" +
779779
" }" +
780-
"}", ContentType.APPLICATION_JSON));
780+
"}");
781781
Response scriptResponse = restClient.performRequest(scriptRequest);
782-
assertEquals(RestStatus.OK.getStatus(), scriptResponse.getStatusLine().getStatusCode());
783782
// end::register-script
783+
assertEquals(RestStatus.OK.getStatus(), scriptResponse.getStatusLine().getStatusCode());
784784

785785
// tag::search-template-request-stored
786786
SearchTemplateRequest request = new SearchTemplateRequest();

‎docs/java-rest/high-level/search/search-template.asciidoc‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ as a string because mustache templates are not always valid JSON.
2424

2525
Search templates can be registered in advance through stored scripts API. Note that
2626
the stored scripts API is not yet available in the high-level REST client, so in this
27-
example we use the simple REST client.
27+
example we use the low-level REST client.
2828

2929
["source","java",subs="attributes,callouts,macros"]
3030
--------------------------------------------------
@@ -61,9 +61,7 @@ include-tagged::{doc-tests}/SearchDocumentationIT.java[search-template-request-o
6161

6262
===== Additional References
6363

64-
The https://www.elastic.co/guide/en/elasticsearch/reference/current/search-template.html#search-template[Search Template HTTP documentation]
65-
contains further examples of how search requests can be templated. For more detailed information on how mustache
66-
works and what can be done with it, please consult the http://mustache.github.io/mustache.5.html[online documentation of the mustache project].
64+
The {ref}/search-template.html[Search Template documentation] contains further examples of how search requests can be templated.
6765

6866
[[java-rest-high-search-template-sync]]
6967
==== Synchronous Execution
@@ -116,3 +114,4 @@ will contain the rendered search source instead of a `SearchResponse`:
116114
--------------------------------------------------
117115
include-tagged::{doc-tests}/SearchDocumentationIT.java[render-search-template-response]
118116
--------------------------------------------------
117+
<1> The rendered source in bytes, in our example `{"query": { "match" : { "title" : "elasticsearch" }}, "size" : 5}`.

‎modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/SearchTemplateRequest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
208208
} else if (scriptType == ScriptType.INLINE) {
209209
builder.field(SOURCE_FIELD.getPreferredName(), script);
210210
} else {
211-
throw new IllegalArgumentException("Unrecognized script type [" + scriptType + "].");
211+
throw new UnsupportedOperationException("Unrecognized script type [" + scriptType + "].");
212212
}
213213

214214
return builder.field(PARAMS_FIELD.getPreferredName(), scriptParams)

‎modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/SearchTemplateRequestTests.java‎

Lines changed: 13 additions & 167 deletions
Original file line numberDiff line numberDiff line change
@@ -19,19 +19,10 @@
1919

2020
package org.elasticsearch.script.mustache;
2121

22-
import org.elasticsearch.common.bytes.BytesReference;
23-
import org.elasticsearch.common.xcontent.ToXContent;
24-
import org.elasticsearch.common.xcontent.XContentBuilder;
25-
import org.elasticsearch.common.xcontent.XContentFactory;
26-
import org.elasticsearch.common.xcontent.XContentParseException;
27-
import org.elasticsearch.common.xcontent.XContentParser;
28-
import org.elasticsearch.common.xcontent.XContentType;
29-
import org.elasticsearch.common.xcontent.json.JsonXContent;
3022
import org.elasticsearch.script.ScriptType;
3123
import org.elasticsearch.search.RandomSearchRequestGenerator;
3224
import org.elasticsearch.search.builder.SearchSourceBuilder;
3325
import org.elasticsearch.test.AbstractStreamableTestCase;
34-
import org.elasticsearch.test.AbstractXContentTestCase;
3526

3627
import java.io.IOException;
3728
import java.util.ArrayList;
@@ -40,11 +31,6 @@
4031
import java.util.Map;
4132
import java.util.function.Consumer;
4233

43-
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertToXContentEquivalent;
44-
import static org.hamcrest.Matchers.equalTo;
45-
import static org.hamcrest.Matchers.hasEntry;
46-
import static org.hamcrest.Matchers.nullValue;
47-
4834
public class SearchTemplateRequestTests extends AbstractStreamableTestCase<SearchTemplateRequest> {
4935

5036
@Override
@@ -54,23 +40,7 @@ protected SearchTemplateRequest createBlankInstance() {
5440

5541
@Override
5642
protected SearchTemplateRequest createTestInstance() {
57-
SearchTemplateRequest request = new SearchTemplateRequest();
58-
request.setScriptType(randomFrom(ScriptType.values()));
59-
request.setScript(randomAlphaOfLength(50));
60-
61-
Map<String, Object> scriptParams = new HashMap<>();
62-
for (int i = 0; i < randomInt(10); i++) {
63-
scriptParams.put(randomAlphaOfLength(5), randomAlphaOfLength(10));
64-
}
65-
request.setScriptParams(scriptParams);
66-
67-
request.setExplain(randomBoolean());
68-
request.setProfile(randomBoolean());
69-
request.setSimulate(randomBoolean());
70-
71-
request.setRequest(RandomSearchRequestGenerator.randomSearchRequest(
72-
SearchSourceBuilder::searchSource));
73-
return request;
43+
return createRandomRequest();
7444
}
7545

7646
@Override
@@ -102,148 +72,24 @@ protected SearchTemplateRequest mutateInstance(SearchTemplateRequest instance) t
10272
return mutatedInstance;
10373
}
10474

105-
public void testToXContentWithInlineTemplate() throws IOException {
106-
SearchTemplateRequest request = new SearchTemplateRequest();
10775

108-
request.setScriptType(ScriptType.INLINE);
109-
request.setScript("{\"query\": { \"match\" : { \"{{my_field}}\" : \"{{my_value}}\" } } }");
110-
request.setProfile(true);
76+
public static SearchTemplateRequest createRandomRequest() {
77+
SearchTemplateRequest request = new SearchTemplateRequest();
78+
request.setScriptType(randomFrom(ScriptType.values()));
79+
request.setScript(randomAlphaOfLength(50));
11180

11281
Map<String, Object> scriptParams = new HashMap<>();
113-
scriptParams.put("my_field", "foo");
114-
scriptParams.put("my_value", "bar");
82+
for (int i = 0; i < randomInt(10); i++) {
83+
scriptParams.put(randomAlphaOfLength(5), randomAlphaOfLength(10));
84+
}
11585
request.setScriptParams(scriptParams);
11686

117-
XContentType contentType = randomFrom(XContentType.values());
118-
XContentBuilder expectedRequest = XContentFactory.contentBuilder(contentType)
119-
.startObject()
120-
.field("source", "{\"query\": { \"match\" : { \"{{my_field}}\" : \"{{my_value}}\" } } }")
121-
.startObject("params")
122-
.field("my_field", "foo")
123-
.field("my_value", "bar")
124-
.endObject()
125-
.field("explain", false)
126-
.field("profile", true)
127-
.endObject();
128-
129-
XContentBuilder actualRequest = XContentFactory.contentBuilder(contentType);
130-
request.toXContent(actualRequest, ToXContent.EMPTY_PARAMS);
131-
132-
assertToXContentEquivalent(BytesReference.bytes(expectedRequest),
133-
BytesReference.bytes(actualRequest),
134-
contentType);
135-
}
136-
137-
public void testToXContentWithStoredTemplate() throws IOException {
138-
SearchTemplateRequest request = new SearchTemplateRequest();
139-
140-
request.setScriptType(ScriptType.STORED);
141-
request.setScript("match_template");
142-
request.setExplain(true);
143-
144-
Map<String, Object> params = new HashMap<>();
145-
params.put("my_field", "foo");
146-
params.put("my_value", "bar");
147-
request.setScriptParams(params);
148-
149-
XContentType contentType = randomFrom(XContentType.values());
150-
XContentBuilder expectedRequest = XContentFactory.contentBuilder(contentType)
151-
.startObject()
152-
.field("id", "match_template")
153-
.startObject("params")
154-
.field("my_field", "foo")
155-
.field("my_value", "bar")
156-
.endObject()
157-
.field("explain", true)
158-
.field("profile", false)
159-
.endObject();
160-
161-
XContentBuilder actualRequest = XContentFactory.contentBuilder(contentType);
162-
request.toXContent(actualRequest, ToXContent.EMPTY_PARAMS);
163-
164-
assertToXContentEquivalent(
165-
BytesReference.bytes(expectedRequest),
166-
BytesReference.bytes(actualRequest),
167-
contentType);
168-
}
169-
170-
/**
171-
* Note that for xContent parsing, we omit two parts of the request:
172-
* - The 'simulate' option is always held constant, since this parameter is not included
173-
* in the request's xContent (it's instead used to determine the request endpoint).
174-
* - We omit the random SearchRequest, since this component only affects the request
175-
* parameters and also isn't captured in the request's xContent.
176-
*/
177-
public void testFromXContent() throws IOException {
178-
AbstractXContentTestCase.testFromXContent(NUMBER_OF_TEST_RUNS,
179-
this::createTestInstanceForXContent,
180-
false,
181-
new String[]{}, field -> false,
182-
this::createParser,
183-
SearchTemplateRequest::fromXContent,
184-
this::assertEqualInstances, true);
185-
}
87+
request.setExplain(randomBoolean());
88+
request.setProfile(randomBoolean());
89+
request.setSimulate(randomBoolean());
18690

187-
private SearchTemplateRequest createTestInstanceForXContent() {
188-
SearchTemplateRequest request = createTestInstance();
189-
request.setSimulate(false);
190-
request.setRequest(null);
91+
request.setRequest(RandomSearchRequestGenerator.randomSearchRequest(
92+
SearchSourceBuilder::searchSource));
19193
return request;
19294
}
193-
194-
public void testFromXContentWithEmbeddedTemplate() throws Exception {
195-
String source = "{" +
196-
" 'source' : {\n" +
197-
" 'query': {\n" +
198-
" 'terms': {\n" +
199-
" 'status': [\n" +
200-
" '{{#status}}',\n" +
201-
" '{{.}}',\n" +
202-
" '{{/status}}'\n" +
203-
" ]\n" +
204-
" }\n" +
205-
" }\n" +
206-
" }" +
207-
"}";
208-
209-
SearchTemplateRequest request = SearchTemplateRequest.fromXContent(newParser(source));
210-
assertThat(request.getScript(), equalTo("{\"query\":{\"terms\":{\"status\":[\"{{#status}}\",\"{{.}}\",\"{{/status}}\"]}}}"));
211-
assertThat(request.getScriptType(), equalTo(ScriptType.INLINE));
212-
assertThat(request.getScriptParams(), nullValue());
213-
}
214-
215-
public void testFromXContentWithEmbeddedTemplateAndParams() throws Exception {
216-
String source = "{" +
217-
" 'source' : {" +
218-
" 'query': { 'match' : { '{{my_field}}' : '{{my_value}}' } }," +
219-
" 'size' : '{{my_size}}'" +
220-
" }," +
221-
" 'params' : {" +
222-
" 'my_field' : 'foo'," +
223-
" 'my_value' : 'bar'," +
224-
" 'my_size' : 5" +
225-
" }" +
226-
"}";
227-
228-
SearchTemplateRequest request = SearchTemplateRequest.fromXContent(newParser(source));
229-
assertThat(request.getScript(), equalTo("{\"query\":{\"match\":{\"{{my_field}}\":\"{{my_value}}\"}},\"size\":\"{{my_size}}\"}"));
230-
assertThat(request.getScriptType(), equalTo(ScriptType.INLINE));
231-
assertThat(request.getScriptParams().size(), equalTo(3));
232-
assertThat(request.getScriptParams(), hasEntry("my_field", "foo"));
233-
assertThat(request.getScriptParams(), hasEntry("my_value", "bar"));
234-
assertThat(request.getScriptParams(), hasEntry("my_size", 5));
235-
}
236-
237-
public void testFromXContentWithMalformedRequest() {
238-
// Unclosed template id
239-
expectThrows(XContentParseException.class, () -> SearchTemplateRequest.fromXContent(newParser("{'id' : 'another_temp }")));
240-
}
241-
242-
/**
243-
* Creates a {@link XContentParser} with the given String while replacing single quote to double quotes.
244-
*/
245-
private XContentParser newParser(String s) throws IOException {
246-
assertNotNull(s);
247-
return createParser(JsonXContent.jsonXContent, s.replace("'", "\""));
248-
}
24995
}

0 commit comments

Comments
 (0)