Make PutStoredScriptRequest immutable (elastic#117556) (elastic#117566

)

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.

modules/lang-mustache/src/internalClusterTest/java/org/elasticsearch/script/mustache/SearchTemplateIT.java

@@ -13,12 +13,10 @@
 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.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 static void assertHitCount(SearchTemplateRequestBuilder requestBuilder, l
     }
 
     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)));
     }
 }

server/src/internalClusterTest/java/org/elasticsearch/script/StoredScriptsIT.java

@@ -11,23 +11,21 @@
 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;
 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 void testBasics() {
             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("""
-                            {"script": {"lang": "%s", "source": "1"} }
-                            """, LANG)), XContentType.JSON),
-                    l
-                )
+                l -> client().execute(TransportPutStoredScriptAction.TYPE, newPutStoredScriptTestRequest("id#", Strings.format("""
+                    {"script": {"lang": "%s", "source": "1"} }
+                    """, LANG)), l)
             ).getMessage()
         );
     }
@@ -91,14 +84,9 @@ public void testMaxScriptSize() {
             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("""
-                            {"script": { "lang": "%s", "source":"0123456789abcdef"} }\
-                            """, LANG)), XContentType.JSON),
-                    l
-                )
+                l -> client().execute(TransportPutStoredScriptAction.TYPE, newPutStoredScriptTestRequest("foobar", Strings.format("""
+                    {"script": { "lang": "%s", "source":"0123456789abcdef"} }\
+                    """, LANG)), l)
             ).getMessage()
         );
     }

server/src/main/java/org/elasticsearch/action/admin/cluster/storedscripts/PutStoredScriptRequest.java

@@ -11,10 +11,12 @@
 
 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 @@
 
 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,25 +49,21 @@ public PutStoredScriptRequest(StreamInput in) throws IOException {
         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
     ) {
         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,31 +76,17 @@ public ActionRequestValidationException validate() {
             validationException = addValidationError("id cannot contain '#' for stored script", validationException);
         }
 
-        if (content == null) {
-            validationException = addValidationError("must specify code for stored script", validationException);
-        }
-
         return validationException;
     }
 
     public String id() {
         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 StoredScriptSource source() {
         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 void writeTo(StreamOutput out) throws IOException {
 
     @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);
     }
 }

server/src/test/java/org/elasticsearch/action/admin/cluster/storedscripts/PutStoredScriptRequestTests.java

@@ -57,9 +57,15 @@ public void testToXContent() throws IOException {
 
         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();

test/framework/src/main/java/org/elasticsearch/action/admin/cluster/storedscripts/StoredScriptIntegTestUtils.java

@@ -11,6 +11,7 @@
 
 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 static void putJsonStoredScript(String id, String jsonContent) {
     }
 
     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)
         );
     }
 }

x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/DlsFlsRequestCacheTests.java

@@ -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.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.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 void testRequestCacheWithTemplateRoleQuery() {
     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();