- Swap client-server integer encoding over to a more standard format (32-bit)

- Unit tests for client code using the new Java support
- Make server auto-shutdown when the client version changes, to avoid stale-server confusion

3 files changed, 104 insertions, 43 deletions

diff --git a/client/src/mill/client/Main.java b/client/src/mill/client/Main.java
index 109a9a9d..3a0a2db8 100644
--- a/client/src/mill/client/Main.java
+++ b/client/src/mill/client/Main.java
@@ -22,7 +22,7 @@ public class Main {
             }
             current = current.getParent();
         }
-        if (ClientServer.isJava9OrAbove) {
+        if (Util.isJava9OrAbove) {
             selfJars.addAll(Arrays.asList(System.getProperty("java.class.path").split(File.pathSeparator)));
         }
         ArrayList<String> l = new java.util.ArrayList<String>();
@@ -90,7 +90,6 @@ public class Main {
         throw new Exception("Reached max process limit: " + 5);
     }
 
-
     public static int run(String lockBase,
                           Runnable initServer,
                           Locks locks,
@@ -101,8 +100,10 @@ public class Main {
                           Map<String, String> env) throws Exception{
 
         FileOutputStream f = new FileOutputStream(lockBase + "/run");
-        ClientServer.writeArgs(System.console() != null, args, f);
-        ClientServer.writeMap(env, f);
+        f.write(System.console() != null ? 1 : 0);
+        Util.writeString(f, System.getProperty("MILL_VERSION"));
+        Util.writeArgs(args, f);
+        Util.writeMap(env, f);
         f.close();
 
         boolean serverInit = false;
@@ -114,15 +115,15 @@ public class Main {
 
         // Need to give sometime for Win32NamedPipeSocket to work
         // if the server is just initialized
-        if (serverInit && ClientServer.isWindows) Thread.sleep(1000);
+        if (serverInit && Util.isWindows) Thread.sleep(1000);
 
         Socket ioSocket = null;
 
         long retryStart = System.currentTimeMillis();
         while(ioSocket == null && System.currentTimeMillis() - retryStart < 1000){
             try{
-                ioSocket = ClientServer.isWindows?
-                        new Win32NamedPipeSocket(ClientServer.WIN32_PIPE_PREFIX + new File(lockBase).getName())
+                ioSocket = Util.isWindows?
+                        new Win32NamedPipeSocket(Util.WIN32_PIPE_PREFIX + new File(lockBase).getName())
                         : new UnixDomainSocket(lockBase + "/io");
             }catch(Throwable e){
                 Thread.sleep(1);
@@ -131,6 +132,7 @@ public class Main {
         if (ioSocket == null){
             throw new Exception("Failed to connect to server");
         }
+
         InputStream outErr = ioSocket.getInputStream();
         OutputStream in = ioSocket.getOutputStream();
         ClientOutputPumper outPump = new ClientOutputPumper(outErr, stdout, stderr);
@@ -205,7 +207,7 @@ class ClientOutputPumper implements Runnable{
                 // instead it throws an IOException whose message contains "ReadFile()".
                 // However, if it throws an IOException before ever reading some bytes,
                 // it could not connect to the server, so exit.
-                if (ClientServer.isWindows && e.getMessage().contains("ReadFile()")) {
+                if (Util.isWindows && e.getMessage().contains("ReadFile()")) {
                     if (first) {
                         System.err.println("Failed to connect to server");
                         System.exit(1);
diff --git a/client/src/mill/client/ClientServer.java b/client/src/mill/client/Util.java
index 468f8ab3..f6adb3cc 100644
--- a/client/src/mill/client/ClientServer.java
+++ b/client/src/mill/client/Util.java
@@ -7,7 +7,7 @@ import java.io.OutputStream;
 import java.util.HashMap;
 import java.util.Map;
 
-public class ClientServer {
+public class Util {
     public static boolean isWindows = System.getProperty("os.name").toLowerCase().startsWith("windows");
     public static boolean isJava9OrAbove = !System.getProperty("java.specification.version").startsWith("1.");
 
@@ -19,22 +19,18 @@ public class ClientServer {
 
     public static String[] parseArgs(InputStream argStream) throws IOException {
 
-        int argsLength = argStream.read();
+        int argsLength = readInt(argStream);
         String[] args = new String[argsLength];
         for (int i = 0; i < args.length; i++) {
             args[i] = readString(argStream);
         }
         return args;
     }
-    public static void writeArgs(Boolean interactive,
-                                 String[] args,
+    public static void writeArgs(String[] args,
                                  OutputStream argStream) throws IOException {
-        argStream.write(interactive ? 1 : 0);
-        argStream.write(args.length);
-        int i = 0;
-        while (i < args.length) {
-            writeString(argStream, args[i]);
-            i += 1;
+        writeInt(argStream, args.length);
+        for(String arg: args){
+            writeString(argStream, arg);
         }
     }
 
@@ -44,7 +40,7 @@ public class ClientServer {
      * does not see the environment changes the client would)
      */
     public static void writeMap(Map<String, String> map, OutputStream argStream) throws IOException {
-        argStream.write(map.size());
+        writeInt(argStream, map.size());
         for (Map.Entry<String, String> kv : map.entrySet()) {
             writeString(argStream, kv.getKey());
             writeString(argStream, kv.getValue());
@@ -53,7 +49,7 @@ public class ClientServer {
 
     public static Map<String, String> parseMap(InputStream argStream) throws IOException {
         Map<String, String> env = new HashMap<>();
-        int mapLength = argStream.read();
+        int mapLength = readInt(argStream);
         for (int i = 0; i < mapLength; i++) {
             String key = readString(argStream);
             String value = readString(argStream);
@@ -62,36 +58,38 @@ public class ClientServer {
         return env;
     }
 
-    private static String readString(InputStream inputStream) throws IOException {
+    public static String readString(InputStream inputStream) throws IOException {
         // Result is between 0 and 255, hence the loop.
-        int read = inputStream.read();
-        int bytesToRead = read;
-        while(read == 255){
-            read = inputStream.read();
-            bytesToRead += read;
-        }
-        byte[] arr = new byte[bytesToRead];
-        int readTotal = 0;
-        while (readTotal < bytesToRead) {
-            read = inputStream.read(arr, readTotal, bytesToRead - readTotal);
-            readTotal += read;
+        int length = readInt(inputStream);
+        byte[] arr = new byte[length];
+        int total = 0;
+        while(total < length){
+            int res = inputStream.read(arr, total, length-total);
+            if (res == -1) throw new IOException("Incomplete String");
+            else{
+                total += res;
+            }
         }
         return new String(arr);
     }
 
-    private static void writeString(OutputStream outputStream, String string) throws IOException {
-        // When written, an int > 255 gets splitted. This logic performs the
-        // split beforehand so that the reading side knows that there is still
-        // more metadata to come before it's able to read the actual data.
-        // Could do with rewriting using logical masks / shifts.
+    public static void writeString(OutputStream outputStream, String string) throws IOException {
         byte[] bytes = string.getBytes();
-        int toWrite = bytes.length;
-        while(toWrite >= 255){
-            outputStream.write(255);
-            toWrite = toWrite - 255;
-        }
-        outputStream.write(toWrite);
+        writeInt(outputStream, bytes.length);
         outputStream.write(bytes);
     }
 
+    public static void writeInt(OutputStream out, int i) throws IOException{
+        out.write((byte)(i >>> 24));
+        out.write((byte)(i >>> 16));
+        out.write((byte)(i >>> 8));
+        out.write((byte)i);
+    }
+    public static int readInt(InputStream in) throws IOException{
+        return ((in.read() & 0xFF) << 24) +
+                ((in.read() & 0xFF) << 16) +
+                ((in.read() & 0xFF) << 8) +
+                (in.read() & 0xFF);
+    }
+
 }
diff --git a/client/test/src/mill/client/ClientTests.java b/client/test/src/mill/client/ClientTests.java
new file mode 100644
index 00000000..4059e5c7
--- /dev/null
+++ b/client/test/src/mill/client/ClientTests.java
@@ -0,0 +1,61 @@
+package mill.client;
+
+import static org.junit.Assert.assertEquals;
+import org.junit.Test;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+
+public class ClientTests {
+    @Test
+    public void readWriteInt() throws Exception{
+        int[] examples = {
+                0, 1, 126, 127, 128, 254, 255, 256, 1024, 99999, 1234567,
+                Integer.MAX_VALUE, Integer.MAX_VALUE / 2, Integer.MIN_VALUE
+        };
+        for(int example0: examples){
+            for(int example: new int[]{-example0, example0}){
+                ByteArrayOutputStream o = new ByteArrayOutputStream();
+                Util.writeInt(o, example);
+                ByteArrayInputStream i = new ByteArrayInputStream(o.toByteArray());
+                int s = Util.readInt(i);
+                assertEquals(example, s);
+                assertEquals(i.available(), 0);
+            }
+        }
+    }
+    @Test
+    public void readWriteString() throws Exception{
+        String[] examples = {
+                "",
+                "hello",
+                "i am cow",
+                "i am cow\nhear me moo\ni weight twice as much as you",
+                "我是一个叉烧包",
+        };
+        for(String example: examples){
+            checkStringRoundTrip(example);
+        }
+    }
+
+    @Test
+    public void readWriteBigString() throws Exception{
+        int[] lengths = {0, 1, 126, 127, 128, 254, 255, 256, 1024, 99999, 1234567};
+        for(int i = 0; i < lengths.length; i++){
+            final char[] bigChars = new char[lengths[i]];
+            java.util.Arrays.fill(bigChars, 'X');
+            checkStringRoundTrip(new String(bigChars));
+        }
+    }
+
+    public void checkStringRoundTrip(String example) throws Exception{
+        ByteArrayOutputStream o = new ByteArrayOutputStream();
+        Util.writeString(o, example);
+        ByteArrayInputStream i = new ByteArrayInputStream(o.toByteArray());
+        String s = Util.readString(i);
+        assertEquals(example, s);
+        assertEquals(i.available(), 0);
+    }
+
+
+}
\ No newline at end of file