More intuitive way to add port bindings: new abstraction "PortBinding"

The use of Ports for adding port bindings requires knowledge of the internal data structure used for storing port bindings in Docker API. This commit adds a more intuitive alternative that works like the --publish option of the Docker CLI and some builder methods like withBinds(Bind...) and withLinks(Link...).
docker-java · marcuslinke · Nov 5, 2014 · Oct 27, 2014 · Oct 31, 2014 · Oct 31, 2014
commit 97bf5ef4aebb0c04b67238e034477b14e8cc2d95
diff --git a/src/main/java/com/github/dockerjava/api/command/StartContainerCmd.java b/src/main/java/com/github/dockerjava/api/command/StartContainerCmd.java
@@ -6,6 +6,7 @@
 import com.github.dockerjava.api.model.Device;
 import com.github.dockerjava.api.model.Link;
 import com.github.dockerjava.api.model.LxcConf;
+import com.github.dockerjava.api.model.PortBinding;
 import com.github.dockerjava.api.model.Ports;
 import com.github.dockerjava.api.model.RestartPolicy;
 
@@ -53,8 +54,21 @@ public interface StartContainerCmd extends DockerCmd<Void> {
 
 	public StartContainerCmd withLxcConf(LxcConf... lxcConf);
 
+	/**
+	 * Add the port bindings that are contained in the given {@link Ports}
+	 * object.
+	 * 
+	 * @see #withPortBindings(PortBinding...)
+	 */
 	public StartContainerCmd withPortBindings(Ports portBindings);
 
+	/**
+	 * Add one or more {@link PortBinding}s.
+	 * This corresponds to the <code>--publish</code> (<code>-p</code>)
+	 * option of the <code>docker run</code> CLI command.
+	 */
+	public StartContainerCmd withPortBindings(PortBinding... portBindings);
+
 	public StartContainerCmd withPrivileged(boolean privileged);
 
 	public StartContainerCmd withPublishAllPorts(boolean publishAllPorts);

diff --git a/src/main/java/com/github/dockerjava/api/model/PortBinding.java b/src/main/java/com/github/dockerjava/api/model/PortBinding.java
@@ -0,0 +1,36 @@
+package com.github.dockerjava.api.model;
+
+import com.github.dockerjava.api.command.InspectContainerResponse.HostConfig;
+import com.github.dockerjava.api.command.InspectContainerResponse.NetworkSettings;
+import com.github.dockerjava.api.model.Ports.Binding;
+
+/**
+ * In a {@link PortBinding}, a network socket on the Docker host, expressed 
+ * as a {@link Binding}, is bound to an {@link ExposedPort} of a container.
+ * A {@link PortBinding} corresponds to the <code>--publish</code>
+ * (<code>-p</code>) option of the <code>docker run</code> (and similar)
+ * CLI command for adding port bindings to a container.
+ * <p>
+ * <i>Note: This is an abstraction used for creating new port bindings.
+ * It is not to be confused with the abstraction used for querying existing
+ * port bindings from a container configuration in 
+ * {@link NetworkSettings#getPorts()} and {@link HostConfig#getPortBindings()}.
+ * In that context, a <code>Map&lt;ExposedPort, Binding[]&gt;</code> is used.</i>
+ */
+public class PortBinding {
+	private final Binding binding;
+	private final ExposedPort exposedPort;
+
+	public PortBinding(Binding binding, ExposedPort exposedPort) {
+		this.binding = binding;
+		this.exposedPort = exposedPort;
+	}
+
+	public Binding getBinding() {
+		return binding;
+	}
+
+	public ExposedPort getExposedPort() {
+		return exposedPort;
+	}
+}
diff --git a/src/main/java/com/github/dockerjava/api/model/Ports.java b/src/main/java/com/github/dockerjava/api/model/Ports.java
@@ -28,6 +28,11 @@
 /**
  * A container for port bindings, made available as a {@link Map} via its
  * {@link #getBindings()} method.
+ * <p> 
+ * <i>Note: This is an abstraction used for querying existing port bindings from 
+ * a container configuration.
+ * It is not to be confused with the {@link PortBinding} abstraction used for
+ * adding new port bindings to a container.</i>
  * 
  * @see HostConfig#getPortBindings()
  * @see NetworkSettings#getPorts()
@@ -38,18 +43,42 @@ public class Ports {
 
     private final Map<ExposedPort, Binding[]> ports = new HashMap<ExposedPort, Binding[]>();
 
+    /**
+     * Creates a {@link Ports} object with no {@link PortBinding}s.
+     * Use {@link #bind(ExposedPort, Binding)} or {@link #add(PortBinding...)}
+     * to add {@link PortBinding}s.
+     */
     public Ports() { }
 
+    /**
+     * Creates a {@link Ports} object with an initial {@link PortBinding} for 
+     * the specified {@link ExposedPort} and {@link Binding}.
+     * Use {@link #bind(ExposedPort, Binding)} or {@link #add(PortBinding...)}
+     * to add more {@link PortBinding}s.
+     */
     public Ports(ExposedPort exposedPort, Binding host) {
     	bind(exposedPort, host);
     }
 
-    public void bind(ExposedPort exposedPort, Binding host) {
+    /**
+     * Adds a new {@link PortBinding} for the specified {@link ExposedPort} and
+     * {@link Binding} to the current bindings.
+     */
+    public void bind(ExposedPort exposedPort, Binding binding) {
         if (ports.containsKey(exposedPort)) {
             Binding[] bindings = ports.get(exposedPort);
-            ports.put(exposedPort, (Binding[]) ArrayUtils.add(bindings, host));
+            ports.put(exposedPort, (Binding[]) ArrayUtils.add(bindings, binding));
         } else {
-            ports.put(exposedPort, new Binding[]{host});
+            ports.put(exposedPort, new Binding[]{binding});
+        }
+    }
+
+    /**
+     * Adds the specified {@link PortBinding}(s) to the list of {@link PortBinding}s. 
+     */
+    public void add(PortBinding... portBindings) {
+        for (PortBinding binding : portBindings) {
+            bind(binding.getExposedPort(), binding.getBinding());
         }
     }
 
@@ -59,6 +88,9 @@ public String toString(){
     }
 
     /**
+     * Returns the port bindings in the format used by the Docker remote API,
+     * i.e. the {@link Binding}s grouped by {@link ExposedPort}.
+     * 
      * @return the port bindings as a {@link Map} that contains one or more
      *         {@link Binding}s per {@link ExposedPort}.
      */
@@ -75,10 +107,12 @@ public static Binding Binding(int hostPort) {
 
 
     /**
-     * The host part of a port binding.
-     * In a port binding a container port, expressed as an {@link ExposedPort},
-     * is published as a port of the Docker host.
+     * A {@link Binding} represents a socket on the Docker host that is
+     * used in a {@link PortBinding}.
+     * It is characterized by an {@link #getHostIp() IP address} and a
+     * {@link #getHostPort() port number}.
      * 
+     * @see Ports#bind(ExposedPort, Binding)
      * @see ExposedPort
      */
     public static class Binding {
@@ -88,7 +122,8 @@ public static class Binding {
         private final int hostPort;
 
         /**
-         * Creates the host part of a port binding.
+         * Creates a {@link Binding} for the given {@link #getHostIp() IP address}
+         * and {@link #getHostPort() port number}.
          * 
          * @see Ports#bind(ExposedPort, Binding)
          * @see ExposedPort
@@ -98,6 +133,13 @@ public Binding(String hostIp, int hostPort) {
             this.hostPort = hostPort;
         }
 
+        /**
+         * Creates a {@link Binding} for the given {@link #getHostPort() port number},
+         * leaving the {@link #getHostIp() IP address} undefined.
+         * 
+         * @see Ports#bind(ExposedPort, Binding)
+         * @see ExposedPort
+         */
         public Binding(int hostPort) {
             this("", hostPort);
         }

diff --git a/src/main/java/com/github/dockerjava/core/command/StartContainerCmdImpl.java b/src/main/java/com/github/dockerjava/core/command/StartContainerCmdImpl.java
@@ -13,6 +13,7 @@
 import com.github.dockerjava.api.model.Link;
 import com.github.dockerjava.api.model.Links;
 import com.github.dockerjava.api.model.LxcConf;
+import com.github.dockerjava.api.model.PortBinding;
 import com.github.dockerjava.api.model.Ports;
 import com.github.dockerjava.api.model.RestartPolicy;
 import com.google.common.base.Preconditions;
@@ -179,6 +180,16 @@ public StartContainerCmd withPortBindings(Ports portBindings) {
 		return this;
 	}
 
+	@Override
+	public StartContainerCmd withPortBindings(PortBinding... portBindings) {
+		Preconditions.checkNotNull(portBindings, "portBindings was not specified");
+		if (this.portBindings == null) {
+			this.portBindings = new Ports();
+		}
+		this.portBindings.add(portBindings);
+		return this;
+	}
+
 	@Override
 	public StartContainerCmd withPrivileged(boolean privileged) {
 		this.privileged = privileged;

diff --git a/...ithub/dockerjava/api/model/PortsTest.java → ...java/api/model/Ports_SerializingTest.java b/...ithub/dockerjava/api/model/PortsTest.java → ...java/api/model/Ports_SerializingTest.java
@@ -9,7 +9,7 @@
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.github.dockerjava.api.model.Ports.Binding;
 
-public class PortsTest {
+public class Ports_SerializingTest {
 	private final ObjectMapper objectMapper = new ObjectMapper();
 	private final String jsonWithDoubleBindingForOnePort = 
 			"{\"80/tcp\":[{\"HostIp\":\"10.0.0.1\",\"HostPort\":\"80\"},{\"HostIp\":\"10.0.0.2\",\"HostPort\":\"80\"}]}";

diff --git a/src/test/java/com/github/dockerjava/api/model/Ports_addBindingsTest.java b/src/test/java/com/github/dockerjava/api/model/Ports_addBindingsTest.java
@@ -0,0 +1,57 @@
+package com.github.dockerjava.api.model;
+
+import static org.testng.Assert.assertEquals;
+
+import java.util.Map;
+
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import com.github.dockerjava.api.model.Ports.Binding;
+
+/**
+ * As there may be several {@link Binding}s per {@link ExposedPort},
+ * it makes a difference if you add {@link PortBinding}s for the
+ * same or different {@link ExposedPort}s to {@link Ports}.
+ * This test verifies that the Map in {@link Ports} is populated 
+ * correctly in both cases.
+ */
+public class Ports_addBindingsTest {
+	private static final ExposedPort TCP_80 = ExposedPort.tcp(80);
+	private static final ExposedPort TCP_90 = ExposedPort.tcp(90);
+	private static final Binding BINDING_8080 = Ports.Binding(8080);
+	private static final Binding BINDING_9090 = Ports.Binding(9090);
+
+	private Ports ports;
+
+	@BeforeMethod
+	public void setup() {
+		ports = new Ports();
+	}
+
+	@Test
+	public void addTwoBindingsForDifferentExposedPorts() {
+		ports.add(
+				new PortBinding(BINDING_8080, TCP_80), 
+				new PortBinding(BINDING_9090, TCP_90));
+
+		Map<ExposedPort, Binding[]> bindings = ports.getBindings();
+		// two keys with one value each
+		assertEquals(bindings.size(), 2);
+		assertEquals(bindings.get(TCP_80), new Binding[] { BINDING_8080 });
+		assertEquals(bindings.get(TCP_90), new Binding[] { BINDING_9090 });
+	}
+
+	@Test
+	public void addTwoBindingsForSameExposedPort() {
+		ports.add(
+				new PortBinding(BINDING_8080, TCP_80), 
+				new PortBinding(BINDING_9090, TCP_80));
+
+		Map<ExposedPort, Binding[]> bindings = ports.getBindings();
+		// one key with two values
+		assertEquals(bindings.size(), 1);
+		assertEquals(bindings.get(TCP_80), new Binding[] { BINDING_8080, BINDING_9090 });
+	}
+
+}