Remove obsolete fields from PutStoredScriptRequest (#123860)

Author: thecoop

server/src/main/java/org/elasticsearch/TransportVersions.java

@@ -206,6 +206,7 @@ static TransportVersion def(int id) {
     public static final TransportVersion ESQL_SERIALIZE_SOURCE_FUNCTIONS_WARNINGS = def(9_016_0_00);
     public static final TransportVersion ESQL_DRIVER_NODE_DESCRIPTION = def(9_017_0_00);
     public static final TransportVersion MULTI_PROJECT = def(9_018_0_00);
+    public static final TransportVersion STORED_SCRIPT_CONTENT_LENGTH = def(9_019_0_00);
 
     /*
      * STOP! READ THIS FIRST! No, really,

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

@@ -9,16 +9,17 @@
 
 package org.elasticsearch.action.admin.cluster.storedscripts;
 
+import org.elasticsearch.TransportVersions;
 import org.elasticsearch.action.ActionRequestValidationException;
 import org.elasticsearch.action.support.master.AcknowledgedRequest;
 import org.elasticsearch.common.Strings;
+import org.elasticsearch.common.bytes.BytesArray;
 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.core.UpdateForV9;
 import org.elasticsearch.script.StoredScriptSource;
 import org.elasticsearch.xcontent.ToXContentFragment;
 import org.elasticsearch.xcontent.XContentBuilder;
@@ -37,27 +38,21 @@ public class PutStoredScriptRequest extends AcknowledgedRequest<PutStoredScriptR
     @Nullable
     private final String context;
 
-    /*
-     * [NOTE: unused fields #117566]
-     * As of #117566 (8.18) the content and xContentType fields are basically unused, except that we use content().length() for some
-     * validation. However, in earlier 8.x versions they did at least influence the output of toString(). That means in 9.x we can replace
-     * these fields with an int representing the original content length once the 9.x transport protocol can diverge from the 8.x one. For
-     * BwC with 8.18 we can simply send any BytesReference of the appropriate length.
-     */
-
-    @UpdateForV9(owner = UpdateForV9.Owner.CORE_INFRA) // see [NOTE: unused fields #117566]
-    private final BytesReference content;
-
-    @UpdateForV9(owner = UpdateForV9.Owner.CORE_INFRA) // see [NOTE: unused fields #117566]
-    private final XContentType xContentType;
+    private final int contentLength;
 
     private final StoredScriptSource source;
 
     public PutStoredScriptRequest(StreamInput in) throws IOException {
         super(in);
         id = in.readOptionalString();
-        content = in.readBytesReference();
-        xContentType = in.readEnum(XContentType.class);
+        if (in.getTransportVersion().onOrAfter(TransportVersions.STORED_SCRIPT_CONTENT_LENGTH)) {
+            contentLength = in.readVInt();
+        } else {
+            BytesReference content = in.readBytesReference();
+            contentLength = content.length();
+
+            in.readEnum(XContentType.class);    // and drop
+        }
         context = in.readOptionalString();
         source = new StoredScriptSource(in);
     }
@@ -67,15 +62,13 @@ public PutStoredScriptRequest(
         TimeValue ackTimeout,
         @Nullable String id,
         @Nullable String context,
-        BytesReference content,
-        XContentType xContentType,
+        int contentLength,
         StoredScriptSource source
     ) {
         super(masterNodeTimeout, ackTimeout);
         this.id = id;
         this.context = context;
-        this.content = Objects.requireNonNull(content);
-        this.xContentType = Objects.requireNonNull(xContentType);
+        this.contentLength = contentLength;
         this.source = Objects.requireNonNull(source);
     }
 
@@ -100,12 +93,8 @@ public String context() {
         return context;
     }
 
-    public BytesReference content() {
-        return content;
-    }
-
-    public XContentType xContentType() {
-        return xContentType;
+    public int contentLength() {
+        return contentLength;
     }
 
     public StoredScriptSource source() {
@@ -116,8 +105,13 @@ public StoredScriptSource source() {
     public void writeTo(StreamOutput out) throws IOException {
         super.writeTo(out);
         out.writeOptionalString(id);
-        out.writeBytesReference(content);
-        XContentHelper.writeTo(out, xContentType);
+        if (out.getTransportVersion().onOrAfter(TransportVersions.STORED_SCRIPT_CONTENT_LENGTH)) {
+            out.writeVInt(contentLength);
+        } else {
+            // generate a bytes reference of the correct size (the content isn't actually used in 8.18)
+            out.writeBytesReference(new BytesArray(new byte[contentLength]));
+            XContentHelper.writeTo(out, XContentType.JSON); // value not actually used by 8.18
+        }
         out.writeOptionalString(context);
         source.writeTo(out);
     }

server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestPutStoredScriptAction.java

@@ -54,8 +54,7 @@ public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client
             getAckTimeout(request),
             request.param("id"),
             request.param("context"),
-            content,
-            request.getXContentType(),
+            content.length(),
             StoredScriptSource.parse(content, xContentType)
         );
         return channel -> client.execute(

server/src/main/java/org/elasticsearch/script/ScriptService.java

@@ -687,12 +687,12 @@ public void putStoredScript(
         PutStoredScriptRequest request,
         ActionListener<AcknowledgedResponse> listener
     ) {
-        if (request.content().length() > maxSizeInBytes) {
+        if (request.contentLength() > maxSizeInBytes) {
             throw new IllegalArgumentException(
                 "exceeded max allowed stored script size in bytes ["
                     + maxSizeInBytes
                     + "] with size ["
-                    + request.content().length()
+                    + request.contentLength()
                     + "] for script ["
                     + request.id()
                     + "]"

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

@@ -9,7 +9,6 @@
 
 package org.elasticsearch.action.admin.cluster.storedscripts;
 
-import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.io.stream.BytesStreamOutput;
 import org.elasticsearch.common.io.stream.StreamInput;
@@ -22,6 +21,8 @@
 import java.io.IOException;
 import java.util.Collections;
 
+import static org.hamcrest.Matchers.equalTo;
+
 public class PutStoredScriptRequestTests extends ESTestCase {
 
     public void testSerialization() throws IOException {
@@ -30,18 +31,15 @@ public void testSerialization() throws IOException {
             TEST_REQUEST_TIMEOUT,
             "bar",
             "context",
-            new BytesArray("{}"),
-            XContentType.JSON,
+            0,
             new StoredScriptSource("foo", "bar", Collections.emptyMap())
         );
 
-        assertEquals(XContentType.JSON, storedScriptRequest.xContentType());
         try (BytesStreamOutput output = new BytesStreamOutput()) {
             storedScriptRequest.writeTo(output);
 
             try (StreamInput in = output.bytes().streamInput()) {
                 PutStoredScriptRequest serialized = new PutStoredScriptRequest(in);
-                assertEquals(XContentType.JSON, serialized.xContentType());
                 assertEquals(storedScriptRequest.id(), serialized.id());
                 assertEquals(storedScriptRequest.context(), serialized.context());
             }
@@ -62,8 +60,7 @@ public void testToXContent() throws IOException {
             TEST_REQUEST_TIMEOUT,
             "test1",
             null,
-            expectedRequestBody,
-            xContentType,
+            expectedRequestBody.length(),
             StoredScriptSource.parse(expectedRequestBody, xContentType)
         );
 
@@ -73,7 +70,6 @@ public void testToXContent() throws IOException {
         requestBuilder.endObject();
 
         BytesReference actualRequestBody = BytesReference.bytes(requestBuilder);
-
-        assertEquals(expectedRequestBody, actualRequestBody);
+        assertThat(actualRequestBody.length(), equalTo(expectedRequestBody.length()));
     }
 }

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

@@ -39,8 +39,7 @@ public static PutStoredScriptRequest newPutStoredScriptTestRequest(String id, By
             TEST_REQUEST_TIMEOUT,
             id,
             null,
-            jsonContent,
-            XContentType.JSON,
+            jsonContent.length(),
             StoredScriptSource.parse(jsonContent, XContentType.JSON)
         );
     }