Make PutStoredScriptRequest immutable (#117556)

No need for this request to be mutable, we always know all the values at
creation time. Also adjusts the `toString()` impl to use the `source`
field, since this is the only spot that we use the `content` so with
this change we can follow up with a 9.x-only change to remove it.
This commit is contained in:
David Turner 2024-11-26 13:58:54 +00:00 committed by GitHub
parent 1495c550ad
commit 2bc1b4f606
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 60 additions and 106 deletions

View file

@ -13,12 +13,10 @@ import org.elasticsearch.action.admin.cluster.storedscripts.DeleteStoredScriptRe
import org.elasticsearch.action.admin.cluster.storedscripts.GetStoredScriptAction;
import org.elasticsearch.action.admin.cluster.storedscripts.GetStoredScriptRequest;
import org.elasticsearch.action.admin.cluster.storedscripts.GetStoredScriptResponse;
import org.elasticsearch.action.admin.cluster.storedscripts.PutStoredScriptRequest;
import org.elasticsearch.action.admin.cluster.storedscripts.TransportDeleteStoredScriptAction;
import org.elasticsearch.action.admin.cluster.storedscripts.TransportPutStoredScriptAction;
import org.elasticsearch.action.bulk.BulkRequestBuilder;
import org.elasticsearch.action.search.SearchRequest;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.script.ScriptType;
@ -39,6 +37,7 @@ import java.util.List;
import java.util.Map;
import java.util.concurrent.ExecutionException;
import static org.elasticsearch.action.admin.cluster.storedscripts.StoredScriptIntegTestUtils.newPutStoredScriptTestRequest;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertResponse;
import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder;
@ -467,12 +466,6 @@ public class SearchTemplateIT extends ESSingleNodeTestCase {
}
private void putJsonStoredScript(String id, String jsonContent) {
assertAcked(
safeExecute(
TransportPutStoredScriptAction.TYPE,
new PutStoredScriptRequest(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT).id(id)
.content(new BytesArray(jsonContent), XContentType.JSON)
)
);
assertAcked(safeExecute(TransportPutStoredScriptAction.TYPE, newPutStoredScriptTestRequest(id, jsonContent)));
}
}

View file

@ -11,16 +11,13 @@ package org.elasticsearch.script;
import org.elasticsearch.action.admin.cluster.storedscripts.DeleteStoredScriptRequest;
import org.elasticsearch.action.admin.cluster.storedscripts.GetStoredScriptAction;
import org.elasticsearch.action.admin.cluster.storedscripts.GetStoredScriptRequest;
import org.elasticsearch.action.admin.cluster.storedscripts.PutStoredScriptRequest;
import org.elasticsearch.action.admin.cluster.storedscripts.TransportDeleteStoredScriptAction;
import org.elasticsearch.action.admin.cluster.storedscripts.TransportPutStoredScriptAction;
import org.elasticsearch.action.support.master.AcknowledgedResponse;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.core.Strings;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.test.ESIntegTestCase;
import org.elasticsearch.xcontent.XContentType;
import java.util.Arrays;
import java.util.Collection;
@ -28,6 +25,7 @@ import java.util.Collections;
import java.util.Map;
import java.util.function.Function;
import static org.elasticsearch.action.admin.cluster.storedscripts.StoredScriptIntegTestUtils.newPutStoredScriptTestRequest;
import static org.elasticsearch.action.admin.cluster.storedscripts.StoredScriptIntegTestUtils.putJsonStoredScript;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
@ -73,14 +71,9 @@ public class StoredScriptsIT extends ESIntegTestCase {
safeAwaitAndUnwrapFailure(
IllegalArgumentException.class,
AcknowledgedResponse.class,
l -> client().execute(
TransportPutStoredScriptAction.TYPE,
new PutStoredScriptRequest(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT).id("id#")
.content(new BytesArray(Strings.format("""
l -> client().execute(TransportPutStoredScriptAction.TYPE, newPutStoredScriptTestRequest("id#", Strings.format("""
{"script": {"lang": "%s", "source": "1"} }
""", LANG)), XContentType.JSON),
l
)
""", LANG)), l)
).getMessage()
);
}
@ -91,14 +84,9 @@ public class StoredScriptsIT extends ESIntegTestCase {
safeAwaitAndUnwrapFailure(
IllegalArgumentException.class,
AcknowledgedResponse.class,
l -> client().execute(
TransportPutStoredScriptAction.TYPE,
new PutStoredScriptRequest(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT).id("foobar")
.content(new BytesArray(Strings.format("""
l -> client().execute(TransportPutStoredScriptAction.TYPE, newPutStoredScriptTestRequest("foobar", Strings.format("""
{"script": { "lang": "%s", "source":"0123456789abcdef"} }\
""", LANG)), XContentType.JSON),
l
)
""", LANG)), l)
).getMessage()
);
}

View file

@ -11,10 +11,12 @@ package org.elasticsearch.action.admin.cluster.storedscripts;
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.action.support.master.AcknowledgedRequest;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.core.TimeValue;
import org.elasticsearch.script.StoredScriptSource;
import org.elasticsearch.xcontent.ToXContentFragment;
@ -28,11 +30,15 @@ import static org.elasticsearch.action.ValidateActions.addValidationError;
public class PutStoredScriptRequest extends AcknowledgedRequest<PutStoredScriptRequest> implements ToXContentFragment {
private String id;
private String context;
private BytesReference content;
private XContentType xContentType;
private StoredScriptSource source;
@Nullable
private final String id;
@Nullable
private final String context;
private final BytesReference content;
private final XContentType xContentType;
private final StoredScriptSource source;
public PutStoredScriptRequest(StreamInput in) throws IOException {
super(in);
@ -43,15 +49,11 @@ public class PutStoredScriptRequest extends AcknowledgedRequest<PutStoredScriptR
source = new StoredScriptSource(in);
}
public PutStoredScriptRequest(TimeValue masterNodeTimeout, TimeValue ackTimeout) {
super(masterNodeTimeout, ackTimeout);
}
public PutStoredScriptRequest(
TimeValue masterNodeTimeout,
TimeValue ackTimeout,
String id,
String context,
@Nullable String id,
@Nullable String context,
BytesReference content,
XContentType xContentType,
StoredScriptSource source
@ -59,9 +61,9 @@ public class PutStoredScriptRequest extends AcknowledgedRequest<PutStoredScriptR
super(masterNodeTimeout, ackTimeout);
this.id = id;
this.context = context;
this.content = content;
this.content = Objects.requireNonNull(content);
this.xContentType = Objects.requireNonNull(xContentType);
this.source = source;
this.source = Objects.requireNonNull(source);
}
@Override
@ -74,10 +76,6 @@ public class PutStoredScriptRequest extends AcknowledgedRequest<PutStoredScriptR
validationException = addValidationError("id cannot contain '#' for stored script", validationException);
}
if (content == null) {
validationException = addValidationError("must specify code for stored script", validationException);
}
return validationException;
}
@ -85,20 +83,10 @@ public class PutStoredScriptRequest extends AcknowledgedRequest<PutStoredScriptR
return id;
}
public PutStoredScriptRequest id(String id) {
this.id = id;
return this;
}
public String context() {
return context;
}
public PutStoredScriptRequest context(String context) {
this.context = context;
return this;
}
public BytesReference content() {
return content;
}
@ -111,16 +99,6 @@ public class PutStoredScriptRequest extends AcknowledgedRequest<PutStoredScriptR
return source;
}
/**
* Set the script source and the content type of the bytes.
*/
public PutStoredScriptRequest content(BytesReference content, XContentType xContentType) {
this.content = content;
this.xContentType = Objects.requireNonNull(xContentType);
this.source = StoredScriptSource.parse(content, xContentType);
return this;
}
@Override
public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
@ -133,28 +111,16 @@ public class PutStoredScriptRequest extends AcknowledgedRequest<PutStoredScriptR
@Override
public String toString() {
String source = "_na_";
try {
source = XContentHelper.convertToJson(content, false, xContentType);
} catch (Exception e) {
// ignore
}
return "put stored script {id ["
+ id
+ "]"
+ (context != null ? ", context [" + context + "]" : "")
+ ", content ["
+ source
+ "]}";
return Strings.format(
"put stored script {id [%s]%s, content [%s]}",
id,
context != null ? ", context [" + context + "]" : "",
source
);
}
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.field("script");
source.toXContent(builder, params);
return builder;
return builder.field("script", source, params);
}
}

View file

@ -57,9 +57,15 @@ public class PutStoredScriptRequestTests extends ESTestCase {
BytesReference expectedRequestBody = BytesReference.bytes(builder);
PutStoredScriptRequest request = new PutStoredScriptRequest(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT);
request.id("test1");
request.content(expectedRequestBody, xContentType);
PutStoredScriptRequest request = new PutStoredScriptRequest(
TEST_REQUEST_TIMEOUT,
TEST_REQUEST_TIMEOUT,
"test1",
null,
expectedRequestBody,
xContentType,
StoredScriptSource.parse(expectedRequestBody, xContentType)
);
XContentBuilder requestBuilder = XContentBuilder.builder(xContentType.xContent());
requestBuilder.startObject();

View file

@ -11,6 +11,7 @@ package org.elasticsearch.action.admin.cluster.storedscripts;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.script.StoredScriptSource;
import org.elasticsearch.test.ESIntegTestCase;
import org.elasticsearch.xcontent.XContentType;
@ -25,11 +26,22 @@ public class StoredScriptIntegTestUtils {
}
public static void putJsonStoredScript(String id, BytesReference jsonContent) {
assertAcked(
ESIntegTestCase.safeExecute(
TransportPutStoredScriptAction.TYPE,
new PutStoredScriptRequest(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT).id(id).content(jsonContent, XContentType.JSON)
)
assertAcked(ESIntegTestCase.safeExecute(TransportPutStoredScriptAction.TYPE, newPutStoredScriptTestRequest(id, jsonContent)));
}
public static PutStoredScriptRequest newPutStoredScriptTestRequest(String id, String jsonContent) {
return newPutStoredScriptTestRequest(id, new BytesArray(jsonContent));
}
public static PutStoredScriptRequest newPutStoredScriptTestRequest(String id, BytesReference jsonContent) {
return new PutStoredScriptRequest(
TEST_REQUEST_TIMEOUT,
TEST_REQUEST_TIMEOUT,
id,
null,
jsonContent,
XContentType.JSON,
StoredScriptSource.parse(jsonContent, XContentType.JSON)
);
}
}

View file

@ -8,13 +8,11 @@
package org.elasticsearch.integration;
import org.elasticsearch.ElasticsearchSecurityException;
import org.elasticsearch.action.admin.cluster.storedscripts.PutStoredScriptRequest;
import org.elasticsearch.action.admin.cluster.storedscripts.TransportPutStoredScriptAction;
import org.elasticsearch.action.admin.indices.alias.Alias;
import org.elasticsearch.action.search.SearchRequestBuilder;
import org.elasticsearch.action.support.broadcast.BroadcastResponse;
import org.elasticsearch.client.internal.Client;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.settings.SecureString;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.core.Strings;
@ -24,7 +22,6 @@ import org.elasticsearch.script.mustache.MustachePlugin;
import org.elasticsearch.search.SearchHit;
import org.elasticsearch.test.SecuritySingleNodeTestCase;
import org.elasticsearch.test.hamcrest.ElasticsearchAssertions;
import org.elasticsearch.xcontent.XContentType;
import org.elasticsearch.xpack.core.XPackSettings;
import org.elasticsearch.xpack.core.security.action.apikey.CreateApiKeyAction;
import org.elasticsearch.xpack.core.security.action.apikey.CreateApiKeyRequest;
@ -43,6 +40,7 @@ import java.util.Set;
import java.util.concurrent.ExecutionException;
import java.util.stream.Collectors;
import static org.elasticsearch.action.admin.cluster.storedscripts.StoredScriptIntegTestUtils.newPutStoredScriptTestRequest;
import static org.elasticsearch.action.support.WriteRequest.RefreshPolicy.IMMEDIATE;
import static org.elasticsearch.action.support.WriteRequest.RefreshPolicy.NONE;
import static org.elasticsearch.action.support.WriteRequest.RefreshPolicy.WAIT_UNTIL;
@ -350,17 +348,8 @@ public class DlsFlsRequestCacheTests extends SecuritySingleNodeTestCase {
private void prepareIndices() {
final Client client = client();
assertAcked(
safeExecute(
TransportPutStoredScriptAction.TYPE,
new PutStoredScriptRequest(TEST_REQUEST_TIMEOUT, TEST_REQUEST_TIMEOUT).id("my-script")
.content(
new BytesArray("""
{"script":{"source":"{\\"match\\":{\\"username\\":\\"{{_user.username}}\\"}}","lang":"mustache"}}"""),
XContentType.JSON
)
)
);
assertAcked(safeExecute(TransportPutStoredScriptAction.TYPE, newPutStoredScriptTestRequest("my-script", """
{"script":{"source":"{\\"match\\":{\\"username\\":\\"{{_user.username}}\\"}}","lang":"mustache"}}""")));
assertAcked(indicesAdmin().prepareCreate(DLS_INDEX).addAlias(new Alias("dls-alias")).get());
client.prepareIndex(DLS_INDEX).setId("101").setSource("number", 101, "letter", "A").get();