Add more tests, fix.

KostyaSha · KostyaSha · commit 52f4c4e1540d · 2016-06-08T01:29:04.000+03:00
diff --git a/src/main/java/com/github/dockerjava/api/model/Device.java b/src/main/java/com/github/dockerjava/api/model/Device.java
@@ -14,6 +14,7 @@
 import javax.annotation.Nonnull;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.StringTokenizer;
 
 @JsonInclude(Include.NON_NULL)
 public class Device {
@@ -59,10 +60,16 @@ public static Device parse(@Nonnull String deviceStr) {
         String src = "";
         String dst = "";
         String permissions = "rwm";
-        final String[] arr = deviceStr.split(":");
-        switch (arr.length) {
+        final String[] arr = deviceStr.trim().split(":");
+        // java String.split() returns wrong length, use tokenizer instead
+        switch (new StringTokenizer(deviceStr, ":").countTokens()) {
             case 3: {
-                permissions = arr[2];
+                // Mismatches docker code logic. While there is no validations after parsing, checking heregit
+                if (validDeviceMode(arr[2])) {
+                    permissions = arr[2];
+                } else {
+                    throw new IllegalArgumentException("Invalid device specification: " + deviceStr);
+                }
             }
             case 2: {
                 if (validDeviceMode(arr[1])) {
@@ -76,7 +83,7 @@ public static Device parse(@Nonnull String deviceStr) {
                 break;
             }
             default: {
-                throw new IllegalArgumentException("invalid device specification: " + deviceStr);
+                throw new IllegalArgumentException("Invalid device specification: " + deviceStr);
             }
         }
 
diff --git a/src/test/java/com/github/dockerjava/api/model/DeviceTest.java b/src/test/java/com/github/dockerjava/api/model/DeviceTest.java
@@ -2,13 +2,61 @@
 
 import org.testng.annotations.Test;
 
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+import static junit.framework.Assert.fail;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
 
 /**
  * @author Kanstantsin Shautsou
  */
 public class DeviceTest {
+
+    public static List<String> validPaths = Arrays.asList(
+            "/home",
+            "/home:/home",
+            "/home:/something/else",
+            "/with space",
+            "/home:/with space",
+            "relative:/absolute-path",
+            "hostPath:/containerPath:r",
+            "/hostPath:/containerPath:rw",
+            "/hostPath:/containerPath:mrw"
+    );
+
+    public static HashMap<String, String> badPaths = new LinkedHashMap<String, String>() {{
+        put("", "bad format for path: ");
+        // TODO implement ValidatePath
+//        put("./", "./ is not an absolute path");
+//        put("../", "../ is not an absolute path");
+//        put("/:../", "../ is not an absolute path");
+//        put("/:path", "path is not an absolute path");
+//        put(":", "bad format for path: :");
+//        put("/tmp:", " is not an absolute path");
+//        put(":test", "bad format for path: :test");
+//        put(":/test", "bad format for path: :/test");
+//        put("tmp:", " is not an absolute path");
+//        put(":test:", "bad format for path: :test:");
+//        put("::", "bad format for path: ::");
+//        put(":::", "bad format for path: :::");
+//        put("/tmp:::", "bad format for path: /tmp:::");
+//        put(":/tmp::", "bad format for path: :/tmp::");
+//        put("path:ro", "ro is not an absolute path");
+//        put("path:rr", "rr is not an absolute path");
+        put("a:/b:ro", "bad mode specified: ro");
+        put("a:/b:rr", "bad mode specified: rr");
+    }};
+
     @Test
     public void testParse() throws Exception {
         assertThat(Device.parse("/dev/sda:/dev/xvdc:r"),
@@ -24,4 +72,24 @@ public void testParse() throws Exception {
                 equalTo(new Device("rw", "/something", "/dev/snd")));
 
     }
+
+    @Test
+    public void testParseBadPaths() {
+        for (Map.Entry<String, String> entry : badPaths.entrySet()) {
+            final String deviceStr = entry.getKey();
+            try {
+                Device.parse(deviceStr);
+                fail("Should fail because: " + entry.getValue() + " '" + deviceStr + "'");
+            } catch (IllegalArgumentException ex) {
+                assertThat(ex.getMessage(), containsString("Invalid device specification:"));
+            }
+        }
+    }
+
+    @Test
+    public void testParseValidPaths() {
+        for (String path : validPaths) {
+            Device.parse(path);
+        }
+    }
 }
