Clean serialization for cli's CommandResponse

* Switch `data` member from Object to `String`
* Compress packages on server so easier to build `data` as `String`
* Move write of `data` member into `encode` method
* Move read of `data` member into ctor

Original commit: elastic/x-pack-elasticsearch@e3a52e7493

sql/cli-proto/src/main/java/org/elasticsearch/xpack/sql/cli/net/protocol/CommandResponse.java

@@ -17,9 +17,9 @@ public class CommandResponse extends Response {
 
     public final long serverTimeQueryReceived, serverTimeResponseSent;
     public final String requestId;
-    public final Object data; // NOCOMMIT should be a string? serialization is weird too.
+    public final String data;
 
-    public CommandResponse(long serverTimeQueryReceived, long serverTimeResponseSent, String requestId, Object data) {
+    public CommandResponse(long serverTimeQueryReceived, long serverTimeResponseSent, String requestId, String data) {
         super(Action.COMMAND);
 
         this.serverTimeQueryReceived = serverTimeQueryReceived;
@@ -34,15 +34,16 @@ public class CommandResponse extends Response {
         serverTimeQueryReceived = in.readLong();
         serverTimeResponseSent = in.readLong();
         requestId = in.readUTF();
-        data = null;
+        data = in.readUTF();
     }
 
     public void encode(DataOutput out) throws IOException {
-        out.writeInt(Status.toSuccess(action)); // NOCOMMIT no symetric!
+        out.writeInt(Status.toSuccess(action)); // NOCOMMIT not symetric!
 
         out.writeLong(serverTimeQueryReceived);
         out.writeLong(serverTimeResponseSent);
         out.writeUTF(requestId);
+        out.writeUTF(data);
     }
 
     @Override

sql/cli-proto/src/test/java/org/elasticsearch/xpack/sql/cli/net/protocol/CommandResponseTests.java

@@ -19,15 +19,9 @@ public class CommandResponseTests extends ESTestCase {
     }
 
     public void testRoundTrip() throws IOException {
-        assertRoundTrip(randomCommandResponse(), (response, out) -> {
+        assertRoundTrip(randomCommandResponse(), CommandResponse::encode, in -> {
-                // NOCOMMIT make this simpler
+                // NOCOMMIT make symmetric
-                response.encode(out);
+                return (CommandResponse) ProtoUtils.readResponse(in, in.readInt());
-                out.writeUTF(response.data.toString());
-            }, in -> {
-                // NOCOMMIT make this simpler
-                CommandResponse response = (CommandResponse) ProtoUtils.readResponse(in, in.readInt());
-                return new CommandResponse(response.serverTimeQueryReceived, response.serverTimeResponseSent,
-                        response.requestId, in.readUTF());
             });
     }
 }

sql/cli/src/main/java/org/elasticsearch/xpack/sql/cli/net/client/CliHttpClient.java

@@ -8,7 +8,6 @@ package org.elasticsearch.xpack.sql.cli.net.client;
 import org.elasticsearch.xpack.sql.cli.CliConfiguration;
 import org.elasticsearch.xpack.sql.cli.CliException;
 import org.elasticsearch.xpack.sql.cli.net.protocol.CommandRequest;
-import org.elasticsearch.xpack.sql.cli.net.protocol.CommandResponse;
 import org.elasticsearch.xpack.sql.cli.net.protocol.InfoRequest;
 import org.elasticsearch.xpack.sql.cli.net.protocol.Proto.Action;
 import org.elasticsearch.xpack.sql.cli.net.protocol.ProtoUtils;
@@ -37,17 +36,7 @@ public class CliHttpClient implements AutoCloseable {
 
     public Response command(String command, String requestId) {
         Bytes ba = http.put(out -> ProtoUtils.write(out, new CommandRequest(command)));
-        return doIO(ba, in -> {
+        return doIO(ba, in -> readResponse(in, Action.COMMAND));
-            Response response = readResponse(in, Action.COMMAND);
-            // read data
-            if (response instanceof CommandResponse) {
-                // NOCOMMIT embed data in response
-                String result = in.readUTF();
-                CommandResponse cr = (CommandResponse) response;
-                return new CommandResponse(cr.serverTimeQueryReceived, cr.serverTimeResponseSent, cr.requestId, result);
-            }
-            return response;
-        });
     }
 
     private static <T> T doIO(Bytes ba, DataInputFunction<T> action) {

sql/cli/src/test/java/org/elasticsearch/xpack/sql/cli/CliProtoHandler.java

@@ -12,8 +12,8 @@ import org.elasticsearch.xpack.sql.TestUtils;
 import org.elasticsearch.xpack.sql.cli.net.protocol.ProtoUtils;
 import org.elasticsearch.xpack.sql.cli.net.protocol.Request;
 import org.elasticsearch.xpack.sql.cli.net.protocol.Response;
-import org.elasticsearch.xpack.sql.plugin.cli.http.CliServerProtoUtils;
+import org.elasticsearch.xpack.sql.server.cli.CliServer;
-import org.elasticsearch.xpack.sql.plugin.cli.server.CliServer;
+import org.elasticsearch.xpack.sql.server.cli.CliServerProtoUtils;
 import org.elasticsearch.xpack.sql.test.server.ProtoHandler;
 
 import java.io.DataInput;

sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlPlugin.java

@@ -24,15 +24,15 @@ import org.elasticsearch.script.ScriptService;
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.watcher.ResourceWatcherService;
 import org.elasticsearch.xpack.sql.execution.PlanExecutor;
-import org.elasticsearch.xpack.sql.plugin.cli.action.CliAction;
-import org.elasticsearch.xpack.sql.plugin.cli.action.TransportCliAction;
-import org.elasticsearch.xpack.sql.plugin.cli.http.CliHttpHandler;
 import org.elasticsearch.xpack.sql.plugin.jdbc.action.JdbcAction;
 import org.elasticsearch.xpack.sql.plugin.jdbc.action.TransportJdbcAction;
 import org.elasticsearch.xpack.sql.plugin.jdbc.http.JdbcHttpHandler;
 import org.elasticsearch.xpack.sql.plugin.sql.action.SqlAction;
 import org.elasticsearch.xpack.sql.plugin.sql.action.TransportSqlAction;
 import org.elasticsearch.xpack.sql.plugin.sql.rest.RestSqlAction;
+import org.elasticsearch.xpack.sql.server.cli.CliAction;
+import org.elasticsearch.xpack.sql.server.cli.CliHttpHandler;
+import org.elasticsearch.xpack.sql.server.cli.TransportCliAction;
 
 import java.util.Arrays;
 import java.util.Collection;

sql/server/src/main/java/org/elasticsearch/xpack/sql/server/cli/CliAction.java

@@ -3,7 +3,7 @@
  * or more contributor license agreements. Licensed under the Elastic License;
  * you may not use this file except in compliance with the Elastic License.
  */
-package org.elasticsearch.xpack.sql.plugin.cli.action;
+package org.elasticsearch.xpack.sql.server.cli;
 
 import org.elasticsearch.action.Action;
 import org.elasticsearch.client.ElasticsearchClient;

sql/server/src/main/java/org/elasticsearch/xpack/sql/server/cli/CliHttpHandler.java

@@ -3,7 +3,7 @@
  * or more contributor license agreements. Licensed under the Elastic License;
  * you may not use this file except in compliance with the Elastic License.
  */
-package org.elasticsearch.xpack.sql.plugin.cli.http;
+package org.elasticsearch.xpack.sql.server.cli;
 
 import java.io.DataInputStream;
 import java.io.IOException;
@@ -17,9 +17,6 @@ import org.elasticsearch.rest.RestChannel;
 import org.elasticsearch.rest.RestController;
 import org.elasticsearch.rest.RestRequest;
 import org.elasticsearch.xpack.sql.cli.net.protocol.ProtoUtils;
-import org.elasticsearch.xpack.sql.plugin.cli.action.CliAction;
-import org.elasticsearch.xpack.sql.plugin.cli.action.CliRequest;
-import org.elasticsearch.xpack.sql.plugin.cli.action.CliResponse;
 import org.elasticsearch.xpack.sql.util.StringUtils;
 
 import static org.elasticsearch.action.ActionListener.wrap;

sql/server/src/main/java/org/elasticsearch/xpack/sql/server/cli/CliRequest.java

@@ -3,7 +3,7 @@
  * or more contributor license agreements. Licensed under the Elastic License;
  * you may not use this file except in compliance with the Elastic License.
  */
-package org.elasticsearch.xpack.sql.plugin.cli.action;
+package org.elasticsearch.xpack.sql.server.cli;
 
 import java.util.Objects;
 

sql/server/src/main/java/org/elasticsearch/xpack/sql/server/cli/CliRequestBuilder.java

@@ -3,7 +3,7 @@
  * or more contributor license agreements. Licensed under the Elastic License;
  * you may not use this file except in compliance with the Elastic License.
  */
-package org.elasticsearch.xpack.sql.plugin.cli.action;
+package org.elasticsearch.xpack.sql.server.cli;
 
 import org.elasticsearch.action.ActionRequestBuilder;
 import org.elasticsearch.client.ElasticsearchClient;

sql/server/src/main/java/org/elasticsearch/xpack/sql/server/cli/CliResponse.java

@@ -3,7 +3,7 @@
  * or more contributor license agreements. Licensed under the Elastic License;
  * you may not use this file except in compliance with the Elastic License.
  */
-package org.elasticsearch.xpack.sql.plugin.cli.action;
+package org.elasticsearch.xpack.sql.server.cli;
 
 import org.elasticsearch.action.ActionResponse;
 import org.elasticsearch.xpack.sql.cli.net.protocol.Response;

sql/server/src/main/java/org/elasticsearch/xpack/sql/server/cli/CliServer.java

@@ -3,7 +3,7 @@
  * or more contributor license agreements. Licensed under the Elastic License;
  * you may not use this file except in compliance with the Elastic License.
  */
-package org.elasticsearch.xpack.sql.plugin.cli.server;
+package org.elasticsearch.xpack.sql.server.cli;
 
 import org.elasticsearch.Build;
 import org.elasticsearch.Version;
@@ -18,7 +18,6 @@ import org.elasticsearch.xpack.sql.cli.net.protocol.Response;
 import org.elasticsearch.xpack.sql.execution.PlanExecutor;
 import org.elasticsearch.xpack.sql.execution.search.SearchHitRowSetCursor;
 import org.elasticsearch.xpack.sql.jdbc.net.protocol.QueryPageRequest;
-import org.elasticsearch.xpack.sql.plugin.cli.http.CliServerProtoUtils;
 import org.elasticsearch.xpack.sql.util.StringUtils;
 
 import java.util.TimeZone;
@@ -71,7 +70,7 @@ public class CliServer {
                         requestId = StringUtils.nullAsEmpty(((SearchHitRowSetCursor) c).scrollId());
                     }
 
-                    listener.onResponse(new CommandResponse(start, stop, requestId, c));
+                    listener.onResponse(new CommandResponse(start, stop, requestId, CliUtils.toString(c)));
                 }, 
                 ex -> listener.onResponse(CliServerProtoUtils.exception(ex, req.action))));
     }

sql/server/src/main/java/org/elasticsearch/xpack/sql/server/cli/CliServerProtoUtils.java

@@ -3,20 +3,18 @@
  * or more contributor license agreements. Licensed under the Elastic License;
  * you may not use this file except in compliance with the Elastic License.
  */
-package org.elasticsearch.xpack.sql.plugin.cli.http;
+package org.elasticsearch.xpack.sql.server.cli;
 
 import org.elasticsearch.ResourceNotFoundException;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.io.stream.BytesStreamOutput;
 import org.elasticsearch.xpack.sql.SqlException;
-import org.elasticsearch.xpack.sql.cli.net.protocol.CommandResponse;
 import org.elasticsearch.xpack.sql.cli.net.protocol.ErrorResponse;
 import org.elasticsearch.xpack.sql.cli.net.protocol.ExceptionResponse;
 import org.elasticsearch.xpack.sql.cli.net.protocol.Proto.Action;
 import org.elasticsearch.xpack.sql.cli.net.protocol.ProtoUtils;
 import org.elasticsearch.xpack.sql.cli.net.protocol.Response;
-import org.elasticsearch.xpack.sql.session.RowSetCursor;
 
 import java.io.DataOutputStream;
 import java.io.IOException;
@@ -31,16 +29,6 @@ public abstract class CliServerProtoUtils {
         try (BytesStreamOutput array = new BytesStreamOutput();
                 DataOutputStream out = new DataOutputStream(array)) {
             ProtoUtils.write(out, response);
-
-            // serialize payload (if present)
-            if (response instanceof CommandResponse) {
-                RowSetCursor cursor = (RowSetCursor) ((CommandResponse) response).data;
-
-                if (cursor != null) {
-                    out.writeUTF(CliUtils.toString(cursor));
-                }
-            }
-
             out.flush();
             return array.bytes();
         }

sql/server/src/main/java/org/elasticsearch/xpack/sql/server/cli/CliUtils.java

@@ -3,7 +3,7 @@
  * or more contributor license agreements. Licensed under the Elastic License;
  * you may not use this file except in compliance with the Elastic License.
  */
-package org.elasticsearch.xpack.sql.plugin.cli.http;
+package org.elasticsearch.xpack.sql.server.cli;
 
 import java.util.concurrent.atomic.AtomicBoolean;
 

sql/server/src/main/java/org/elasticsearch/xpack/sql/server/cli/TransportCliAction.java

@@ -3,7 +3,7 @@
  * or more contributor license agreements. Licensed under the Elastic License;
  * you may not use this file except in compliance with the Elastic License.
  */
-package org.elasticsearch.xpack.sql.plugin.cli.action;
+package org.elasticsearch.xpack.sql.server.cli;
 
 import org.elasticsearch.Build;
 import org.elasticsearch.Version;
@@ -18,7 +18,6 @@ import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.transport.TransportService;
 import org.elasticsearch.xpack.sql.analysis.catalog.EsCatalog;
 import org.elasticsearch.xpack.sql.execution.PlanExecutor;
-import org.elasticsearch.xpack.sql.plugin.cli.server.CliServer;
 
 import static org.elasticsearch.xpack.sql.util.ActionUtils.chain;