Fix serialization bug in painless execute api request (#36075)

The `xContentType` was incorrectly serialized:
`xContentType.mediaTypeWithoutParameters()` was used to serialize, but
`xContentType.mediaType()` was used to de-serialize.

Also the serialization test class did not test `ContextSetup` well,
this was due to limitation in serialization test base class. Changed the
test class to manually test xcontent serialization, so that for both binary
and xcontent serialization tests, the `ContextSetup` is properly tested.

Closes #36050

Author: martijnvg

modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessExecuteAction.java

@@ -225,19 +225,20 @@ public boolean equals(Object o) {
                 ContextSetup that = (ContextSetup) o;
                 return Objects.equals(index, that.index) &&
                     Objects.equals(document, that.document) &&
-                    Objects.equals(query, that.query);
+                    Objects.equals(query, that.query) &&
+                    Objects.equals(xContentType, that.xContentType);
             }
 
             @Override
             public int hashCode() {
-                return Objects.hash(index, document, query);
+                return Objects.hash(index, document, query, xContentType);
             }
 
             @Override
             public void writeTo(StreamOutput out) throws IOException {
                 out.writeOptionalString(index);
                 out.writeOptionalBytesReference(document);
-                out.writeOptionalString(xContentType != null ? xContentType.mediaType(): null);
+                out.writeOptionalString(xContentType != null ? xContentType.mediaTypeWithoutParameters(): null);
                 out.writeOptionalNamedWriteable(query);
             }
 
@@ -354,11 +355,13 @@ public void writeTo(StreamOutput out) throws IOException {
         // For testing only:
         @Override
         public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
+            builder.startObject();
             builder.field(SCRIPT_FIELD.getPreferredName(), script);
             builder.field(CONTEXT_FIELD.getPreferredName(), context.name);
             if (contextSetup != null) {
                 builder.field(CONTEXT_SETUP_FIELD.getPreferredName(), contextSetup);
             }
+            builder.endObject();
             return builder;
         }
 

modules/lang-painless/src/test/java/org/elasticsearch/painless/PainlessExecuteRequestTests.java

@@ -20,22 +20,55 @@
 
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
+import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
 import org.elasticsearch.common.xcontent.NamedXContentRegistry;
+import org.elasticsearch.common.xcontent.XContent;
+import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
+import org.elasticsearch.common.xcontent.XContentType;
 import org.elasticsearch.index.query.MatchAllQueryBuilder;
 import org.elasticsearch.index.query.QueryBuilder;
 import org.elasticsearch.painless.PainlessExecuteAction.Request.ContextSetup;
 import org.elasticsearch.script.Script;
 import org.elasticsearch.script.ScriptContext;
 import org.elasticsearch.script.ScriptType;
 import org.elasticsearch.search.SearchModule;
-import org.elasticsearch.test.AbstractStreamableXContentTestCase;
+import org.elasticsearch.test.AbstractStreamableTestCase;
 
 import java.io.IOException;
+import java.io.UncheckedIOException;
 import java.util.Collections;
 
-public class PainlessExecuteRequestTests extends AbstractStreamableXContentTestCase<PainlessExecuteAction.Request> {
+import static org.hamcrest.Matchers.equalTo;
+
+public class PainlessExecuteRequestTests extends AbstractStreamableTestCase<PainlessExecuteAction.Request> {
+
+    // Testing XContent serialization manually here, because the xContentType field in ContextSetup determines
+    // how the request needs to parse and the xcontent serialization framework randomizes that. The XContentType
+    // is not known and accessable when the test request instance is created in the xcontent serialization framework.
+    // Changing that is a big change. Writing a custom xcontent test here is the best option for now, because as far
+    // as I know this request class is the only case where this is a problem.
+    public final void testFromXContent() throws Exception {
+        for (int i = 0; i < 20; i++) {
+            PainlessExecuteAction.Request testInstance = createTestInstance();
+            ContextSetup contextSetup = testInstance.getContextSetup();
+            XContent xContent = randomFrom(XContentType.values()).xContent();
+            if (contextSetup != null && contextSetup.getXContentType() != null) {
+                xContent = contextSetup.getXContentType().xContent();
+            }
+
+            try (XContentBuilder builder = XContentBuilder.builder(xContent)) {
+                builder.value(testInstance);
+                StreamInput instanceInput = BytesReference.bytes(builder).streamInput();
+                try (XContentParser parser = xContent.createParser(xContentRegistry(), LoggingDeprecationHandler.INSTANCE, instanceInput)) {
+                    PainlessExecuteAction.Request result = PainlessExecuteAction.Request.parse(parser);
+                    assertThat(result, equalTo(testInstance));
+                }
+            }
+        }
+    }
 
     @Override
     protected NamedWriteableRegistry getNamedWriteableRegistry() {
@@ -60,16 +93,6 @@ protected PainlessExecuteAction.Request createBlankInstance() {
         return new PainlessExecuteAction.Request();
     }
 
-    @Override
-    protected PainlessExecuteAction.Request doParseInstance(XContentParser parser) throws IOException {
-        return PainlessExecuteAction.Request.parse(parser);
-    }
-
-    @Override
-    protected boolean supportsUnknownFields() {
-        return false;
-    }
-
     public void testValidate() {
         Script script = new Script(ScriptType.STORED, null, randomAlphaOfLength(10), Collections.emptyMap());
         PainlessExecuteAction.Request request = new PainlessExecuteAction.Request(script, null, null);
@@ -78,20 +101,24 @@ public void testValidate() {
         assertEquals("Validation Failed: 1: only inline scripts are supported;", e.getMessage());
     }
 
-    private static ContextSetup randomContextSetup() {
+    private static ContextSetup randomContextSetup()  {
         String index = randomBoolean() ? randomAlphaOfLength(4) : null;
         QueryBuilder query = randomBoolean() ? new MatchAllQueryBuilder() : null;
-        // TODO: pass down XContextType to createTestInstance() method.
-        // otherwise the document itself is different causing test failures.
-        // This should be done in a separate change as the test instance is created before xcontent type is randomly picked and
-        // all the createTestInstance() methods need to be changed, which will make this a big chnage
-//        BytesReference doc = randomBoolean() ? new BytesArray("{}") : null;
         BytesReference doc = null;
+        XContentType xContentType = randomFrom(XContentType.values());
+        if (randomBoolean()) {
+            try {
+                XContentBuilder xContentBuilder = XContentBuilder.builder(xContentType.xContent());
+                xContentBuilder.startObject();
+                xContentBuilder.endObject();
+                doc = BytesReference.bytes(xContentBuilder);
+            } catch (IOException e) {
+                throw new UncheckedIOException(e);
+            }
+        }
 
         ContextSetup contextSetup = new ContextSetup(index, doc, query);
-//        if (doc != null) {
-//            contextSetup.setXContentType(XContentType.JSON);
-//        }
+        contextSetup.setXContentType(xContentType);
         return contextSetup;
     }
 }