Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add put stored script support to high-level rest client #31323

Merged
merged 11 commits into from
Sep 9, 2018
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
package org.elasticsearch.client;

import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.admin.cluster.storedscripts.PutStoredScriptRequest;
import org.elasticsearch.action.admin.cluster.storedscripts.PutStoredScriptResponse;
import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest;
import org.elasticsearch.action.admin.indices.alias.IndicesAliasesResponse;
import org.elasticsearch.action.admin.indices.alias.get.GetAliasesRequest;
Expand Down Expand Up @@ -689,4 +691,33 @@ public void getTemplateAsync(GetIndexTemplatesRequest getIndexTemplatesRequest,
restHighLevelClient.performRequestAsyncAndParseEntity(getIndexTemplatesRequest, RequestConverters::getTemplates,
options, GetIndexTemplatesResponse::fromXContent, listener, emptySet());
}

/**
* Puts an stored script using the Scripting API.
* See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/6.2/modules-scripting.html"> Scripting API
* on elastic.co</a>
* @param putStoredScriptRequest the request
* @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized
* @return the response
* @throws IOException in case there is a problem sending the request or parsing back the response
*/
public PutStoredScriptResponse putStoredScript(PutStoredScriptRequest putStoredScriptRequest,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this API call doesn't belong to any category and is described at https://github.com/elastic/elasticsearch/blob/master/rest-api-spec/src/main/resources/rest-api-spec/api/put_script.json - and it looks like it has to go to main RestHighLevelClient ( e.g. in comparison to https://github.com/elastic/elasticsearch/blob/master/rest-api-spec/src/main/resources/rest-api-spec/api/indices.create.json that is for category indices)

Could you please move those methods to RestHighLevelClient ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you also remove the stored part from the method names?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should I also remove the stored parts from the class names?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no those have to stay as they are as it's already existing classes that people have been using with the transport client.

RequestOptions options) throws IOException {
return restHighLevelClient.performRequestAndParseEntity(putStoredScriptRequest, RequestConverters::putStoredScript, options,
PutStoredScriptResponse::fromXContent, emptySet());
}

/**
* Asynchronously puts an stored script using the Scripting API.
* See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/6.2/modules-scripting.html"> Scripting API
* on elastic.co</a>
* @param putStoredScriptRequest the request
* @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized
* @param listener the listener to be notified upon request completion
*/
public void putStoredScriptAsync(PutStoredScriptRequest putStoredScriptRequest, RequestOptions options,
ActionListener<PutStoredScriptResponse> listener) {
restHighLevelClient.performRequestAsyncAndParseEntity(putStoredScriptRequest, RequestConverters::putStoredScript, options,
PutStoredScriptResponse::fromXContent, listener, emptySet());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import org.elasticsearch.action.admin.cluster.repositories.put.PutRepositoryRequest;
import org.elasticsearch.action.admin.cluster.repositories.verify.VerifyRepositoryRequest;
import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest;
import org.elasticsearch.action.admin.cluster.storedscripts.PutStoredScriptRequest;
import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest;
import org.elasticsearch.action.admin.indices.alias.get.GetAliasesRequest;
import org.elasticsearch.action.admin.indices.cache.clear.ClearIndicesCacheRequest;
Expand Down Expand Up @@ -65,8 +66,8 @@
import org.elasticsearch.action.get.MultiGetRequest;
import org.elasticsearch.action.index.IndexRequest;
import org.elasticsearch.action.ingest.DeletePipelineRequest;
import org.elasticsearch.action.ingest.PutPipelineRequest;
import org.elasticsearch.action.ingest.GetPipelineRequest;
import org.elasticsearch.action.ingest.PutPipelineRequest;
import org.elasticsearch.action.search.ClearScrollRequest;
import org.elasticsearch.action.search.MultiSearchRequest;
import org.elasticsearch.action.search.SearchRequest;
Expand Down Expand Up @@ -877,6 +878,16 @@ static Request getTemplates(GetIndexTemplatesRequest getIndexTemplatesRequest) t
return request;
}

static Request putStoredScript(PutStoredScriptRequest putStoredScriptRequest) throws IOException {
String endpoint = new EndpointBuilder().addPathPartAsIs("_scripts").addPathPart(putStoredScriptRequest.id()).build();
Request request = new Request(HttpPost.METHOD_NAME, endpoint);
Params params = new Params(request);
params.withTimeout(putStoredScriptRequest.timeout());
params.withMasterTimeout(putStoredScriptRequest.masterNodeTimeout());
request.setEntity(createEntity(putStoredScriptRequest, REQUEST_BODY_CONTENT_TYPE));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't we also support a context parameter?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry for asking that, I do not really understand what does context mean here. After searching documentation I only found two docs:

https://www.elastic.co/guide/en/elasticsearch/painless/master/painless-execute-api.html
https://www.elastic.co/guide/en/elasticsearch/reference/master/modules-scripting-security.html

where should I put the parameter?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't be sorry, I can explain! at REST, we also accept the context query_string parameter. It is a string and PutStoredScriptRequest already supports it, you just have to read it from the request and set the corresponding parameter so that we pass it through to the REST layer.

return request;
}

private static HttpEntity createEntity(ToXContent toXContent, XContentType xContentType) throws IOException {
BytesRef source = XContentHelper.toXContent(toXContent, xContentType, false).toBytesRef();
return new ByteArrayEntity(source.bytes, source.offset, source.length, createContentType(xContentType));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import org.apache.http.client.methods.HttpPut;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.ElasticsearchStatusException;
import org.elasticsearch.action.admin.cluster.storedscripts.PutStoredScriptRequest;
import org.elasticsearch.action.admin.cluster.storedscripts.PutStoredScriptResponse;
import org.elasticsearch.action.admin.indices.alias.Alias;
import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest;
import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest.AliasActions;
Expand Down Expand Up @@ -71,12 +73,14 @@
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.cluster.metadata.IndexTemplateMetaData;
import org.elasticsearch.common.ValidationException;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.ByteSizeUnit;
import org.elasticsearch.common.unit.ByteSizeValue;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.common.xcontent.json.JsonXContent;
import org.elasticsearch.common.xcontent.support.XContentMapValues;
import org.elasticsearch.index.IndexSettings;
Expand Down Expand Up @@ -1201,4 +1205,31 @@ public void testGetIndexTemplate() throws Exception {
new GetIndexTemplatesRequest().names("the-template-*"), client.indices()::getTemplate, client.indices()::getTemplateAsync));
assertThat(notFound.status(), equalTo(RestStatus.NOT_FOUND));
}

public void testPutStoredScript() throws Exception {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the same applicable for tests for this REST API call - RestHighLevelClientTests is better place for it as this call doesn't belong to any category

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it is a good idea to move putStoredScript to other place, but I am not really sure whether RestHighLevelClient is a good place or not.

But according to #27205 it seems that script API should be categorized as Indices API. I am confused...

Copy link
Contributor

@vladimirdolzhenko vladimirdolzhenko Jun 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point...

in fact in #27205 it is under indices api while rest api spec says it's out of any category...

in my PR #31355 I put them into StoredScriptsIT - to group them reasonably...

@javanna do you have any objections / concerns / suggestion ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RestHighLevelClientTests is a unit test while this is an integ test, adding them to a new StoredScriptsIT sounds good to me. I will move the API to the right category in the meta issue, it should not be under indices, the transport client even exposed it up until now under cluster, but we should follow our rest spec so we are aligned with the other language clients.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and I assume the same is applicable for StoredScriptsDocumentationIT

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes that sounds good too

RestHighLevelClient client = highLevelClient();
XContentType xContentType = randomFrom(XContentType.values());
PutStoredScriptRequest request = new PutStoredScriptRequest()
.id("script1");

try (XContentBuilder builder = XContentBuilder.builder(xContentType.xContent())) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be great to encapsulate all that in PutStoredScriptRequest.toXContent(XContentBuilder builder, Params params)

builder.startObject();
builder.startObject("script")
.field("lang", "painless")
.field("source", "Math.log(_score * 2) + params.multiplier")
.endObject();
builder.endObject();
request.content(BytesReference.bytes(builder), xContentType);
}

PutStoredScriptResponse response = execute(request,
client.indices()::putStoredScript, client.indices()::putStoredScriptAsync);
assertThat(response.isAcknowledged(), equalTo(true));

Map<String, Object> script = getAsMap("/_scripts/script1");
assertThat(extractValue("_id", script), equalTo("script1"));
assertThat(extractValue("found", script), equalTo(true));
assertThat(extractValue("script.lang", script), equalTo("painless"));
assertThat(extractValue("script.source", script), equalTo("Math.log(_score * 2) + params.multiplier"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import org.elasticsearch.action.admin.cluster.repositories.put.PutRepositoryRequest;
import org.elasticsearch.action.admin.cluster.repositories.verify.VerifyRepositoryRequest;
import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest;
import org.elasticsearch.action.admin.cluster.storedscripts.PutStoredScriptRequest;
import org.elasticsearch.action.admin.indices.alias.Alias;
import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest;
import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest.AliasActions;
Expand Down Expand Up @@ -1913,6 +1914,34 @@ public void testGetTemplateRequest() throws Exception {
assertThat(request.getEntity(), nullValue());
}

public void testPutStoredScript() throws Exception {
PutStoredScriptRequest putStoredScriptRequest = new PutStoredScriptRequest();

String id = randomAlphaOfLengthBetween(5, 10);
putStoredScriptRequest.id(id);

XContentType xContentType = randomFrom(XContentType.values());
try (XContentBuilder builder = XContentBuilder.builder(xContentType.xContent())) {
builder.startObject();
builder.startObject("script")
.field("lang", "painless")
.field("source", "Math.log(_score * 2) + params.multiplier")
.endObject();
builder.endObject();

putStoredScriptRequest.content(BytesReference.bytes(builder), xContentType);
}

Map<String, String> expectedParams = new HashMap<>();
setRandomMasterTimeout(putStoredScriptRequest, expectedParams);
setRandomTimeout(putStoredScriptRequest::timeout, AcknowledgedRequest.DEFAULT_ACK_TIMEOUT, expectedParams);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here we should test the context param too?

Request request = RequestConverters.putStoredScript(putStoredScriptRequest);

assertThat(request.getEndpoint(), equalTo("/_scripts/" + id));
assertThat(request.getParameters(), equalTo(expectedParams));
assertThat(request.getEntity(), notNullValue());
}

private static void assertToXContentBody(ToXContent expectedBody, HttpEntity actualEntity) throws IOException {
BytesReference expectedBytes = XContentHelper.toXContent(expectedBody, REQUEST_BODY_CONTENT_TYPE, false);
assertEquals(XContentType.JSON.mediaTypeWithoutParameters(), actualEntity.getContentType().getValue());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.LatchedActionListener;
import org.elasticsearch.action.admin.cluster.storedscripts.PutStoredScriptRequest;
import org.elasticsearch.action.admin.cluster.storedscripts.PutStoredScriptResponse;
import org.elasticsearch.action.admin.indices.alias.Alias;
import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest;
import org.elasticsearch.action.admin.indices.alias.IndicesAliasesRequest.AliasActions;
Expand Down Expand Up @@ -73,6 +75,8 @@
import org.elasticsearch.cluster.metadata.AliasMetaData;
import org.elasticsearch.cluster.metadata.IndexTemplateMetaData;
import org.elasticsearch.cluster.metadata.MappingMetaData;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.collect.ImmutableOpenMap;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.ByteSizeUnit;
Expand All @@ -93,6 +97,7 @@
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;

import static org.elasticsearch.common.xcontent.support.XContentMapValues.extractValue;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasSize;

Expand Down Expand Up @@ -2128,4 +2133,110 @@ public void onFailure(Exception e) {

assertTrue(latch.await(30L, TimeUnit.SECONDS));
}

public void testPutStoredScript() throws Exception {
RestHighLevelClient client = highLevelClient();

{
createIndex("index1", Settings.EMPTY);
}

{
// tag::put-stored-script-request
PutStoredScriptRequest request = new PutStoredScriptRequest();
request.id("id"); // <1>
request.content(new BytesArray(
"{\n" +
"\"script\": {\n" +
"\"lang\": \"painless\",\n" +
"\"source\": \"Math.log(_score * 2) + params.multiplier\"" +
"}\n" +
"}\n"
), XContentType.JSON); // <2>
// end::put-stored-script-request
}

{
PutStoredScriptRequest request = new PutStoredScriptRequest();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same - pls move it to PutStoredScriptRequest.toXContent(XContentBuilder builder, Params params)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vladimirdolzhenko Thanks for your advice. But I am curious about that is it necessary to move the code to PutStoredScriptRequest.toXContent(XContentBuilder builder, Params params). I found there are lots of test code written like this.

And as far as I know, toXContent will build a XContentBuilder object based on the content of a request, and it will be used in RequestConverters. So I am not really sure how to put this into PutStoredScriptRequest.toXContent. If you have any idea how to do that, please let me know and I will fix this as soon as possible. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, my fault - I thought there is a method to set source directly - you've already added PutStoredScriptRequest.toXContent that creates nested script object with source itself - so that part is good.

request.id("id");

// tag::put-stored-script-content-painless
XContentBuilder builder = XContentFactory.jsonBuilder();
builder.startObject();
{
builder.startObject("script");
{
builder.field("lang", "painless");
builder.field("source", "Math.log(_score * 2) + params.multiplier");
}
builder.endObject();
}
builder.endObject();
request.content(BytesReference.bytes(builder), XContentType.JSON); // <1>
// end::put-stored-script-content-painless


// tag::put-stored-script-execute
PutStoredScriptResponse putStoredScriptResponse = client.indices().putStoredScript(request, RequestOptions.DEFAULT);
// end::put-stored-script-execute

// tag::put-stored-script-response
boolean acknowledged = putStoredScriptResponse.isAcknowledged(); // <1>
// end::put-stored-script-response

assertTrue(acknowledged);

// tag::put-stored-script-execute-listener
ActionListener<PutStoredScriptResponse> listener =
new ActionListener<PutStoredScriptResponse>() {
@Override
public void onResponse(PutStoredScriptResponse response) {
// <1>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'd rather prefer to store response in some kind of ref and to check it after that

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nevermind - the same style is used in other documentation examples

}

@Override
public void onFailure(Exception e) {
// <2>
}
};
// end::put-stored-script-execute-listener

// Replace the empty listener by a blocking listener in test
final CountDownLatch latch = new CountDownLatch(1);
listener = new LatchedActionListener<>(listener, latch);

// tag::put-stored-script-execute-async
client.indices().putStoredScriptAsync(request, RequestOptions.DEFAULT, listener); // <1>
// end::put-stored-script-execute-async

assertTrue(latch.await(30L, TimeUnit.SECONDS));
}

{
PutStoredScriptRequest request = new PutStoredScriptRequest();
request.id("id");

// tag::put-stored-script-content-mustache
XContentBuilder builder = XContentFactory.jsonBuilder();
builder.startObject();
{
builder.startObject("script");
{
builder.field("lang", "mustache");
builder.field("source", "{\"query\":{\"match\":{\"title\":\"{{query_string}}\"}}}");
}
builder.endObject();
}
builder.endObject();
request.content(BytesReference.bytes(builder), XContentType.JSON); // <1>
// end::put-stored-script-content-mustache

client.indices().putStoredScript(request, RequestOptions.DEFAULT);

Map<String, Object> script = getAsMap("/_scripts/id");
assertThat(extractValue("script.lang", script), equalTo("mustache"));
assertThat(extractValue("script.source", script), equalTo("{\"query\":{\"match\":{\"title\":\"{{query_string}}\"}}}"));
}

}
}
83 changes: 83 additions & 0 deletions docs/java-rest/high-level/indices/put_storedscript.asciidoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
[[java-rest-high-put-stored-script]]
=== Put Stored Script API

[[java-rest-high-put-stored-script-request]]
==== Put Stored Script Request

A `PutStoredScriptRequest` requires an `id` and `content`:

["source","java",subs="attributes,callouts,macros"]
--------------------------------------------------
include-tagged::{doc-tests}/IndicesClientDocumentationIT.java[put-stored-script-request]
--------------------------------------------------
<1> The id of the script
<2> The content of the script

[[java-rest-high-put-stored-script-content]]
==== Content
The content of a script can be written in different languages and provided in
different ways:

["source","java",subs="attributes,callouts,macros"]
--------------------------------------------------
include-tagged::{doc-tests}/IndicesClientDocumentationIT.java[put-stored-script-content-painless]
--------------------------------------------------
<1> Specify a painless script and provided as `XContentBuilder` object.
Note that the builder need to be passed as a `BytesReference` object
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs


["source","java",subs="attributes,callouts,macros"]
--------------------------------------------------
include-tagged::{doc-tests}/IndicesClientDocumentationIT.java[put-stored-script-content-mustache]
--------------------------------------------------
<1> Specify a mustache script and provided as `XContentBuilder` object.
Note that value of source can be direvtly provided as a JSON string
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

directly


[[java-rest-high-put-stored-script-sync]]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that we need to add the Optional parameters section with the 3 optional parameters that are supported

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean context, xContentType and source in PutStoredScriptRequest class?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

more like context, and the two timeouts.

==== Synchronous Execution
["source","java",subs="attributes,callouts,macros"]
--------------------------------------------------
include-tagged::{doc-tests}/IndicesClientDocumentationIT.java[put-stored-script-execute]
--------------------------------------------------

[[java-rest-high-put-stored-script-async]]
==== Asynchronous Execution

The asynchronous execution of a put stored script request requires both the `PutStoredScriptRequest`
instance and an `ActionListener` instance to be passed to the asynchronous method:

["source","java",subs="attributes,callouts,macros"]
--------------------------------------------------
include-tagged::{doc-tests}/IndicesClientDocumentationIT.java[put-stored-script-execute-async]
--------------------------------------------------
<1> The `PutStoredScriptRequest` to execute and the `ActionListener` to use when
the execution completes

[[java-rest-high-put-stored-script-listener]]
===== Action Listener

The asynchronous method does not block and returns immediately. Once it is
completed the `ActionListener` is called back using the `onResponse` method
if the execution successfully completed or using the `onFailure` method if
it failed.

A typical listener for `PutStoredScriptResponse` looks like:

["source","java",subs="attributes,callouts,macros"]
--------------------------------------------------
include-tagged::{doc-tests}/IndicesClientDocumentationIT.java[put-stored-script-execute-listener]
--------------------------------------------------
<1> Called when the execution is successfully completed. The response is
provided as an argument
<2> Called in case of failure. The raised exception is provided as an argument

[[java-rest-high-put-stored-script-response]]
==== Put Stored Script Response

The returned `PutStoredScriptResponse` allows to retrieve information about the
executed operation as follows:

["source","java",subs="attributes,callouts,macros"]
--------------------------------------------------
include-tagged::{doc-tests}/IndicesClientDocumentationIT.java[put-stored-script-response]
--------------------------------------------------
<1> Indicates whether all of the nodes have acknowledged the request
Loading