New enum AccessMode improves instantiation and parsing of Bind

docker-java · marcuslinke · Oct 7, 2014 · Oct 7, 2014 · Oct 7, 2014 · Oct 7, 2014
commit 74ea00bc856b6b5d9d598397d2707c834240abf4
diff --git a/src/main/java/com/github/dockerjava/api/model/AccessMode.java b/src/main/java/com/github/dockerjava/api/model/AccessMode.java
@@ -0,0 +1,19 @@
+package com.github.dockerjava.api.model;
+
+/**
+ * The access mode of a file system or file: <code>read-write</code>
+ * or <code>read-only</code>.
+ */
+public enum AccessMode {
+	/** read-write */
+	rw,
+
+	/** read-only */
+	ro;
+
+	/**
+	 * The default {@link AccessMode}: {@link #rw}
+	 */
+	public static final AccessMode DEFAULT = rw;
+
+}
diff --git a/src/main/java/com/github/dockerjava/api/model/Bind.java b/src/main/java/com/github/dockerjava/api/model/Bind.java
@@ -1,5 +1,8 @@
 package com.github.dockerjava.api.model;
 
+import static com.github.dockerjava.api.model.AccessMode.ro;
+import static com.github.dockerjava.api.model.AccessMode.rw;
+
 import org.apache.commons.lang.builder.EqualsBuilder;
 import org.apache.commons.lang.builder.HashCodeBuilder;
 
@@ -14,16 +17,24 @@ public class Bind {
 
 	private Volume volume;
 
-	private boolean readOnly = false;
+	private AccessMode accessMode;
 
 	public Bind(String path, Volume volume) {
-		this(path, volume, false);
+		this(path, volume, AccessMode.DEFAULT);
 	}
 
-	public Bind(String path, Volume volume, boolean readOnly) {
+	public Bind(String path, Volume volume, AccessMode accessMode) {
 		this.path = path;
 		this.volume = volume;
-		this.readOnly = readOnly;
+		this.accessMode = accessMode;
+	}
+
+	/**
+	 * @deprecated use {@link #Bind(String, Volume, AccessMode)}
+	 */
+	@Deprecated
+	public Bind(String path, Volume volume, boolean readOnly) {
+		this(path, volume, readOnly ? ro : rw);
 	}
 
 	public String getPath() {
@@ -33,9 +44,17 @@ public String getPath() {
 	public Volume getVolume() {
 		return volume;
 	}
+
+	public AccessMode getAccessMode() {
+		return accessMode;
+	}
 
+	/**
+	 * @deprecated use {@link #getAccessMode()}
+	 */
+	@Deprecated
 	public boolean isReadOnly() {
-		return readOnly;
+		return ro.equals(accessMode);
 	}
 
 	/**
@@ -53,12 +72,8 @@ public static Bind parse(String serialized) {
 				return new Bind(parts[0], Volume.parse(parts[1]));
 			}
 			case 3: {
-				if ("rw".equals(parts[2].toLowerCase()))
-					return new Bind(parts[0], Volume.parse(parts[1]), false);
-				else if ("ro".equals(parts[2].toLowerCase()))
-					return new Bind(parts[0], Volume.parse(parts[1]), true);
-				else
-					throw new IllegalArgumentException();
+				AccessMode accessMode = AccessMode.valueOf(parts[2].toLowerCase());
+				return new Bind(parts[0], Volume.parse(parts[1]), accessMode);
 			}
 			default: {
 				throw new IllegalArgumentException();
@@ -76,15 +91,15 @@ public boolean equals(Object obj) {
 			Bind other = (Bind) obj;
 			return new EqualsBuilder().append(path, other.getPath())
 					.append(volume, other.getVolume())
-					.append(readOnly, other.isReadOnly()).isEquals();
+					.append(accessMode, other.getAccessMode()).isEquals();
 		} else
 			return super.equals(obj);
 	}
 
 	@Override
 	public int hashCode() {
 		return new HashCodeBuilder().append(path).append(volume)
-				.append(readOnly).toHashCode();
+				.append(accessMode).toHashCode();
 	}
 
 	/**
@@ -97,7 +112,7 @@ public int hashCode() {
 	 */
 	@Override
 	public String toString() {
-		return path + ":" + volume.toString() + (readOnly ? ":ro" : ":rw");
+		return path + ":" + volume.toString() + ":" + accessMode.toString();
 	}
 
 }
diff --git a/src/test/java/com/github/dockerjava/api/model/AccessModeTest.java b/src/test/java/com/github/dockerjava/api/model/AccessModeTest.java
@@ -0,0 +1,31 @@
+package com.github.dockerjava.api.model;
+
+import static com.github.dockerjava.api.model.AccessMode.rw;
+import static org.testng.Assert.assertEquals;
+
+import org.testng.annotations.Test;
+
+public class AccessModeTest {
+
+	@Test
+	public void defaultAccessMode() {
+		assertEquals(AccessMode.DEFAULT, rw);
+	}
+
+	@Test
+	public void stringify() {
+		assertEquals(AccessMode.rw.toString(), "rw");
+	}
+
+	@Test
+	public void fromString() {
+		assertEquals(AccessMode.valueOf("rw"), rw);
+	}
+
+	@Test(expectedExceptions = IllegalArgumentException.class, 
+			expectedExceptionsMessageRegExp = "No enum constant.*")
+	public void fromIllegalString() {
+		AccessMode.valueOf("xx");
+	}
+
+}
diff --git a/src/test/java/com/github/dockerjava/api/model/BindTest.java b/src/test/java/com/github/dockerjava/api/model/BindTest.java
@@ -1,5 +1,7 @@
 package com.github.dockerjava.api.model;
 
+import static com.github.dockerjava.api.model.AccessMode.ro;
+import static com.github.dockerjava.api.model.AccessMode.rw;
 import static org.testng.Assert.assertEquals;
 
 import org.testng.annotations.Test;
@@ -11,23 +13,23 @@ public void parseUsingDefaultAccessMode() {
 		Bind bind = Bind.parse("/host:/container");
 		assertEquals(bind.getPath(), "/host");
 		assertEquals(bind.getVolume().getPath(), "/container");
-		assertEquals(bind.isReadOnly(), false);
+		assertEquals(bind.getAccessMode(), AccessMode.DEFAULT);
 	}
 
 	@Test
 	public void parseReadWrite() {
 		Bind bind = Bind.parse("/host:/container:rw");
 		assertEquals(bind.getPath(), "/host");
 		assertEquals(bind.getVolume().getPath(), "/container");
-		assertEquals(bind.isReadOnly(), false);
+		assertEquals(bind.getAccessMode(), rw);
 	}
 
 	@Test
 	public void parseReadOnly() {
 		Bind bind = Bind.parse("/host:/container:ro");
 		assertEquals(bind.getPath(), "/host");
 		assertEquals(bind.getVolume().getPath(), "/container");
-		assertEquals(bind.isReadOnly(), true);
+		assertEquals(bind.getAccessMode(), ro);
 	}
 
 	@Test(expectedExceptions = IllegalArgumentException.class,

diff --git a/src/test/java/com/github/dockerjava/core/command/StartContainerCmdImplTest.java b/src/test/java/com/github/dockerjava/core/command/StartContainerCmdImplTest.java
@@ -1,5 +1,6 @@
 package com.github.dockerjava.core.command;
 
+import static com.github.dockerjava.api.model.AccessMode.ro;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.equalTo;
@@ -72,7 +73,7 @@ public void startContainerWithVolumes() throws DockerException {
 		assertThat(inspectContainerResponse.getConfig().getVolumes().keySet(),
 				contains("/opt/webapp1", "/opt/webapp2"));
 
-		dockerClient.startContainerCmd(container.getId()).withBinds(new Bind("/src/webapp1", volume1, true), new Bind("/src/webapp2", volume2)).exec();
+		dockerClient.startContainerCmd(container.getId()).withBinds(new Bind("/src/webapp1", volume1, ro), new Bind("/src/webapp2", volume2)).exec();
 
 		dockerClient.waitContainerCmd(container.getId()).exec();