Start on round trip tests

I don't want to dig too closely into the ES integration yet because
it being fixed by Costin so instead I'll make an example for round
trip testing.

Original commit: elastic/x-pack-elasticsearch@4d8fbc571e

sql-clients/cli/src/test/java/org/elasticsearch/xpack/sql/cli/integration/net/protocol/ProtoTests.java

@@ -26,7 +26,7 @@ import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.Matchers.startsWith;
 
 public class ProtoTests extends ESTestCase {
-    // NOCOMMIT maybe this should be moved to an integration test inside the plugin
+    // NOCOMMIT probably should be an integration test that runs against a running copy of ES with SQL installed
 
     private static Client esClient;
     private static CliHttpServer server;

sql-clients/jdbc-proto/build.gradle

@@ -5,6 +5,7 @@ description = 'Request and response objects shared by the jdbc driver and ' +
 
 dependencies {
   compile project(":x-pack-elasticsearch:sql-clients:net-client")
+  testCompile project(':x-pack-elasticsearch:sql-clients:test-utils')
 }
 
 dependencyLicenses {

sql-clients/jdbc-proto/src/main/java/org/elasticsearch/xpack/sql/jdbc/net/protocol/MetaColumnResponse.java

@@ -33,6 +33,7 @@ public class MetaColumnResponse extends Response {
         out.writeInt(columns.size());
 
         for (MetaColumnInfo info : columns) {
+            // NOCOMMIT core would make MetaColumnInfo know how to read and write itself which feels cleaner. 
             out.writeUTF(info.name);
             out.writeUTF(info.table);
             out.writeInt(info.type);

sql-clients/jdbc-proto/src/main/java/org/elasticsearch/xpack/sql/jdbc/net/protocol/ProtoUtils.java

@@ -54,7 +54,7 @@ public abstract class ProtoUtils {
             case META_COLUMN:
                 return MetaColumnRequest.decode(in);
             case QUERY_INIT:
-                return QueryInitRequest.decode(in);
+                return new QueryInitRequest(in);
             case QUERY_PAGE:
                 return QueryPageRequest.decode(in);
             default:
@@ -85,6 +85,7 @@ public abstract class ProtoUtils {
                         return QueryPageResponse.decode(in);
                     default:
                         // cannot find action type
+                        // NOCOMMIT it feels like this should throw *something*
                         return null;
                 }
             default:

sql-clients/jdbc-proto/src/main/java/org/elasticsearch/xpack/sql/jdbc/net/protocol/QueryInitRequest.java

@@ -9,6 +9,7 @@ import java.io.DataInput;
 import java.io.DataOutput;
 import java.io.IOException;
 import java.util.Locale;
+import java.util.Objects;
 
 import org.elasticsearch.xpack.sql.jdbc.net.protocol.Proto.Action;
 
@@ -27,6 +28,13 @@ public class QueryInitRequest extends Request {
         this.timeout = timeout;
     }
 
+    QueryInitRequest(DataInput in) throws IOException {
+        super(Action.QUERY_INIT);
+        fetchSize = in.readInt();
+        timeout = new TimeoutInfo(in);
+        query = in.readUTF();
+    }
+
     @Override
     public void encode(DataOutput out) throws IOException {
         out.writeInt(action.value());
@@ -35,16 +43,24 @@ public class QueryInitRequest extends Request {
         out.writeUTF(query);
     }
 
-    public static QueryInitRequest decode(DataInput in) throws IOException {
-        int fetchSize = in.readInt();
-        TimeoutInfo timeout = TimeoutInfo.readTimeout(in);
-        String query = in.readUTF();
-
-        return new QueryInitRequest(fetchSize, query, timeout);
-    }
-
     @Override
     public String toString() {
         return format(Locale.ROOT, "SqlInitReq[%s]", query);
     }
+
+    @Override
+    public boolean equals(Object obj) {
+        if (obj == null || obj.getClass() != getClass()) {
+            return false;
+        }
+        QueryInitRequest other = (QueryInitRequest) obj;
+        return fetchSize == other.fetchSize
+                && Objects.equals(query, other.query)
+                && Objects.equals(timeout, other.timeout);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(fetchSize, query, timeout);
+    }
 }

sql-clients/jdbc-proto/src/main/java/org/elasticsearch/xpack/sql/jdbc/net/protocol/QueryPageRequest.java

@@ -34,7 +34,7 @@ public class QueryPageRequest extends Request {
 
     public static QueryPageRequest decode(DataInput in) throws IOException {
         String requestId = in.readUTF();
-        TimeoutInfo timeout = TimeoutInfo.readTimeout(in);
+        TimeoutInfo timeout = new TimeoutInfo(in);
         return new QueryPageRequest(requestId, timeout);
     }
 

sql-clients/jdbc-proto/src/main/java/org/elasticsearch/xpack/sql/jdbc/net/protocol/TimeoutInfo.java

@@ -8,9 +8,10 @@ package org.elasticsearch.xpack.sql.jdbc.net.protocol;
 import java.io.DataInput;
 import java.io.DataOutput;
 import java.io.IOException;
+import java.util.Objects;
 
 public class TimeoutInfo {
-
+    // NOCOMMIT javadoc on each of these would be nice because I don't know from reading how "timeout" is different from requestTimeout
     public final long clientTime, timeout, requestTimeout;
 
     public TimeoutInfo(long clientTime, long timeout, long requestTimeout) {
@@ -19,17 +20,36 @@ public class TimeoutInfo {
         this.requestTimeout = requestTimeout;
     }
 
+    TimeoutInfo(DataInput in) throws IOException {
+        clientTime = in.readLong();
+        timeout = in.readLong();
+        requestTimeout = in.readLong();
+    }
+
     void encode(DataOutput out) throws IOException {
         out.writeLong(clientTime);
         out.writeLong(timeout);
         out.writeLong(requestTimeout);
     }
 
-    static TimeoutInfo readTimeout(DataInput in) throws IOException {
+    @Override
-        long clientTime = in.readLong();
+    public String toString() {
-        long timeout = in.readLong();
+        return "client=" + clientTime + ",timeout=" + timeout + ",request=" + requestTimeout;
-        long requestTimeout = in.readLong();
+    }
 
-        return new TimeoutInfo(clientTime, timeout, requestTimeout);
+    @Override
+    public boolean equals(Object obj) {
+        if (obj == null || obj.getClass() != getClass()) {
+            return false;
+        }
+        TimeoutInfo other = (TimeoutInfo) obj;
+        return clientTime == other.clientTime
+                && timeout == other.timeout
+                && requestTimeout == other.requestTimeout;
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(clientTime, timeout, requestTimeout);
     }
 }

sql-clients/jdbc-proto/src/test/java/org/elasticsearch/xpack/sql/jdbc/net/protocol/QueryInitRequestTests.java

@@ -0,0 +1,25 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * 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.jdbc.net.protocol;
+
+import org.elasticsearch.test.ESTestCase;
+
+import java.io.IOException;
+
+import static org.elasticsearch.xpack.sql.jdbc.net.protocol.TimeoutInfoTests.randomTimeoutInfo;
+import static org.elasticsearch.xpack.sql.test.RoundTripTestUtils.assertRoundTrip;
+
+public class QueryInitRequestTests extends ESTestCase {
+    public static QueryInitRequest randomQueryInitRequest() {
+        return new QueryInitRequest(between(0, Integer.MAX_VALUE), randomAlphaOfLength(5), randomTimeoutInfo());
+    }
+
+    public void testRoundTrip() throws IOException {
+        TimeoutInfo example = new TimeoutInfo(randomNonNegativeLong(), randomNonNegativeLong(), randomNonNegativeLong());
+        assertRoundTrip(example, TimeoutInfo::encode, TimeoutInfo::new);
+    }
+
+}

sql-clients/jdbc-proto/src/test/java/org/elasticsearch/xpack/sql/jdbc/net/protocol/TimeoutInfoTests.java

@@ -0,0 +1,22 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * 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.jdbc.net.protocol;
+
+import org.elasticsearch.test.ESTestCase;
+
+import java.io.IOException;
+
+import static org.elasticsearch.xpack.sql.test.RoundTripTestUtils.assertRoundTrip;
+
+public class TimeoutInfoTests extends ESTestCase {
+    static TimeoutInfo randomTimeoutInfo() {
+        return new TimeoutInfo(randomNonNegativeLong(), randomNonNegativeLong(), randomNonNegativeLong());
+    }
+
+    public void testRoundTrip() throws IOException {
+        assertRoundTrip(randomTimeoutInfo(), TimeoutInfo::encode, TimeoutInfo::new);
+    }
+}

sql-clients/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/integration/net/protocol/ProtoTests.java

@@ -40,7 +40,7 @@ import static org.hamcrest.Matchers.startsWith;
 
 
 public class ProtoTests extends ESTestCase {
-    // NOCOMMIT maybe this should be moved to an integration test inside the plugin
+    // NOCOMMIT probably should be an integration test that runs against a running copy of ES with SQL installed
 
     private static Client esClient;
     private static JdbcHttpServer server;

sql-clients/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/integration/query/CompareToH2BaseTestCase.java

@@ -9,7 +9,6 @@ import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.io.PathUtils;
 import org.elasticsearch.test.ESTestCase;
 import org.junit.Assert;
-import org.junit.Test;
 
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -25,6 +24,7 @@ import static java.lang.String.format;
 import static org.elasticsearch.xpack.sql.jdbc.integration.util.JdbcAssert.assertResultSets;
 
 public abstract class CompareToH2BaseTestCase extends ESTestCase {
+    // NOCOMMIT subclasses should probably all be integration tests running against a running Elasticsearch
     public final String queryName;
     public final String query;
     public final Integer lineNumber;

sql-clients/test-utils/build.gradle

@@ -4,6 +4,8 @@ description = 'Shared test utilities for jdbc and cli projects'
 
 dependencies {
   compile "org.elasticsearch.client:transport:${version}"
+  compile "junit:junit:${versions.junit}"
+  compile "org.hamcrest:hamcrest-all:${versions.hamcrest}"
 }
 
 /* Elasticsearch traditionally disables this for test utilities because it is

sql-clients/test-utils/src/main/java/org/elasticsearch/xpack/sql/test/RoundTripTestUtils.java

@@ -0,0 +1,48 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * 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.test;
+
+import org.elasticsearch.common.CheckedBiConsumer;
+import org.elasticsearch.common.CheckedFunction;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.DataInput;
+import java.io.DataInputStream;
+import java.io.DataOutput;
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * Base class for testing round trips across the serialization protocol.
+ */
+public abstract class RoundTripTestUtils {
+    private RoundTripTestUtils () {
+        // Only static utilities here
+    }
+
+    public static <T> void assertRoundTrip(T example, CheckedBiConsumer<T, DataOutput, IOException> encode,
+            CheckedFunction<DataInput, T, IOException> decode) throws IOException {
+        T once = roundTrip(example, encode, decode);
+        assertEquals(example, once);
+        T twice = roundTrip(once, encode, decode);
+        assertEquals(example, twice);
+        assertEquals(once, twice);
+    }
+
+    public static <T> T roundTrip(T example, CheckedBiConsumer<T, DataOutput, IOException> encode,
+            CheckedFunction<DataInput, T, IOException> decode) throws IOException {
+        try (ByteArrayOutputStream out = new ByteArrayOutputStream()) {
+            encode.accept(example, new DataOutputStream(out));
+            try (InputStream in = new ByteArrayInputStream(out.toByteArray())) {
+                return decode.apply(new DataInputStream(in));
+            }
+        }
+    }
+}