From 1c91db0dd95a68c861edd6d1abf53030095ee18e Mon Sep 17 00:00:00 2001 From: Harald Albers Date: Tue, 7 Oct 2014 12:16:32 +0200 Subject: [PATCH 1/2] Use Bind.toString() in serialization This also changes the serialization in Binds.Serializer to always include the access mode. This is no problem in Docker API. --- .../com/github/dockerjava/api/model/Bind.java | 19 +++++++++++++++++++ .../github/dockerjava/api/model/Binds.java | 5 +---- .../github/dockerjava/api/model/Volume.java | 12 ++++++++++++ .../github/dockerjava/api/model/BindTest.java | 15 +++++++++++++++ .../dockerjava/api/model/VolumeTest.java | 12 ++++++++++++ 5 files changed, 59 insertions(+), 4 deletions(-) create mode 100644 src/test/java/com/github/dockerjava/api/model/VolumeTest.java diff --git a/src/main/java/com/github/dockerjava/api/model/Bind.java b/src/main/java/com/github/dockerjava/api/model/Bind.java index 0c4dd3150..ffae75f6b 100644 --- a/src/main/java/com/github/dockerjava/api/model/Bind.java +++ b/src/main/java/com/github/dockerjava/api/model/Bind.java @@ -3,6 +3,11 @@ import org.apache.commons.lang.builder.EqualsBuilder; import org.apache.commons.lang.builder.HashCodeBuilder; +/** + * Represents a host path being bind mounted as a {@link Volume} + * in a Docker container. + * The Bind can be in read only or read write access mode. + */ public class Bind { private String path; @@ -81,4 +86,18 @@ public int hashCode() { return new HashCodeBuilder().append(path).append(volume) .append(readOnly).toHashCode(); } + + /** + * Returns a string representation of this {@link Bind} suitable + * for inclusion in a JSON message. + * The format is <host path>:<container path>:<access mode>, + * like the argument in {@link #parse(String)}. + * + * @return a string representation of this {@link Bind} + */ + @Override + public String toString() { + return path + ":" + volume.toString() + (readOnly ? ":ro" : ":rw"); + } + } diff --git a/src/main/java/com/github/dockerjava/api/model/Binds.java b/src/main/java/com/github/dockerjava/api/model/Binds.java index 5b69edaba..bfc8dbf2d 100644 --- a/src/main/java/com/github/dockerjava/api/model/Binds.java +++ b/src/main/java/com/github/dockerjava/api/model/Binds.java @@ -44,10 +44,7 @@ public void serialize(Binds binds, JsonGenerator jsonGen, // jsonGen.writeStartArray(); for (Bind bind : binds.getBinds()) { - String s = bind.getPath() + ":" + bind.getVolume().toString(); - if(bind.isReadOnly()) s += ":ro"; - jsonGen.writeString(s); - + jsonGen.writeString(bind.toString()); } jsonGen.writeEndArray(); // diff --git a/src/main/java/com/github/dockerjava/api/model/Volume.java b/src/main/java/com/github/dockerjava/api/model/Volume.java index 50f62a27b..1a1890147 100644 --- a/src/main/java/com/github/dockerjava/api/model/Volume.java +++ b/src/main/java/com/github/dockerjava/api/model/Volume.java @@ -19,6 +19,11 @@ import com.fasterxml.jackson.databind.annotation.JsonSerialize; import com.fasterxml.jackson.databind.node.NullNode; +/** + * Represents a bind mounted volume in a Docker container. + * + * @see Bind + */ @JsonDeserialize(using = Volume.Deserializer.class) @JsonSerialize(using = Volume.Serializer.class) public class Volume { @@ -43,6 +48,13 @@ public static Volume parse(String serialized) { return new Volume(serialized); } + /** + * Returns a string representation of this {@link Volume} suitable + * for inclusion in a JSON message. + * The returned String is simply the container path, {@link #getPath()}. + * + * @return a string representation of this {@link Volume} + */ @Override public String toString() { return getPath(); diff --git a/src/test/java/com/github/dockerjava/api/model/BindTest.java b/src/test/java/com/github/dockerjava/api/model/BindTest.java index 412c537fa..bf96987e7 100644 --- a/src/test/java/com/github/dockerjava/api/model/BindTest.java +++ b/src/test/java/com/github/dockerjava/api/model/BindTest.java @@ -48,4 +48,19 @@ public void parseNull() { Bind.parse(null); } + @Test + public void toStringReadOnly() { + assertEquals(Bind.parse("/host:/container:ro").toString(), "/host:/container:ro"); + } + + @Test + public void toStringReadWrite() { + assertEquals(Bind.parse("/host:/container:rw").toString(), "/host:/container:rw"); + } + + @Test + public void toStringDefaultAccessMode() { + assertEquals(Bind.parse("/host:/container").toString(), "/host:/container:rw"); + } + } diff --git a/src/test/java/com/github/dockerjava/api/model/VolumeTest.java b/src/test/java/com/github/dockerjava/api/model/VolumeTest.java new file mode 100644 index 000000000..8fdf19975 --- /dev/null +++ b/src/test/java/com/github/dockerjava/api/model/VolumeTest.java @@ -0,0 +1,12 @@ +package com.github.dockerjava.api.model; + +import static org.testng.Assert.assertEquals; + +import org.testng.annotations.Test; + +public class VolumeTest { + @Test + public void stringify() { + assertEquals(Volume.parse("/path").toString(), "/path"); + } +} From 74ea00bc856b6b5d9d598397d2707c834240abf4 Mon Sep 17 00:00:00 2001 From: Harald Albers Date: Tue, 7 Oct 2014 13:29:37 +0200 Subject: [PATCH 2/2] New enum AccessMode improves instantiation and parsing of Bind --- .../dockerjava/api/model/AccessMode.java | 19 ++++++++ .../com/github/dockerjava/api/model/Bind.java | 43 +++++++++++++------ .../dockerjava/api/model/AccessModeTest.java | 31 +++++++++++++ .../github/dockerjava/api/model/BindTest.java | 8 ++-- .../command/StartContainerCmdImplTest.java | 3 +- 5 files changed, 86 insertions(+), 18 deletions(-) create mode 100644 src/main/java/com/github/dockerjava/api/model/AccessMode.java create mode 100644 src/test/java/com/github/dockerjava/api/model/AccessModeTest.java diff --git a/src/main/java/com/github/dockerjava/api/model/AccessMode.java b/src/main/java/com/github/dockerjava/api/model/AccessMode.java new file mode 100644 index 000000000..e01065368 --- /dev/null +++ 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: read-write + * or read-only. + */ +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 index ffae75f6b..2a838c279 100644 --- 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,7 +91,7 @@ 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); } @@ -84,7 +99,7 @@ public boolean equals(Object 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 new file mode 100644 index 000000000..d93289580 --- /dev/null +++ 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 index bf96987e7..50a41fc38 100644 --- 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,7 +13,7 @@ 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 @@ -19,7 +21,7 @@ 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 @@ -27,7 +29,7 @@ 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 index a45333326..2c72c444d 100644 --- 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();