refactor!: exposed and mapped ports api (#656)

Follow-up PR after #655 (kudos to @estigma88 🎉 )

- rename `ExposedPort` to `ContainerPort`
  - minor internal renaming to align terminology
- `with_mapped_port`: accept two arguments  instead of tuple
- introduce `IntoContainerPort` to make a shortcut for conversion from
`u16` to `ContainerPort`

testcontainers/src/core.rs

@@ -2,7 +2,7 @@ pub use self::{
     containers::*,
     image::{CmdWaitFor, ContainerState, ExecCommand, Image, ImageExt, WaitFor},
     mounts::{AccessMode, Mount, MountType},
-    ports::ExposedPort,
+    ports::{ContainerPort, IntoContainerPort},
 };
 
 mod image;

testcontainers/src/core/containers/async_container.rs

@@ -14,7 +14,7 @@ use crate::{
         macros,
         network::Network,
         ports::Ports,
-        ContainerState, ExecCommand, ExposedPort, WaitFor,
+        ContainerPort, ContainerState, ExecCommand, WaitFor,
     },
     ContainerRequest, Image,
 };
@@ -92,7 +92,7 @@ where
     ///
     /// This method does **not** magically expose the given port, it simply performs a mapping on
     /// the already exposed ports. If a docker container does not expose a port, this method will return an error.
-    pub async fn get_host_port_ipv4(&self, internal_port: ExposedPort) -> Result<u16> {
+    pub async fn get_host_port_ipv4(&self, internal_port: ContainerPort) -> Result<u16> {
         self.ports()
             .await?
             .map_to_host_port_ipv4(internal_port)
@@ -107,7 +107,7 @@ where
     ///
     /// This method does **not** magically expose the given port, it simply performs a mapping on
     /// the already exposed ports. If a docker container does not expose a port, this method will return an error.
-    pub async fn get_host_port_ipv6(&self, internal_port: ExposedPort) -> Result<u16> {
+    pub async fn get_host_port_ipv6(&self, internal_port: ContainerPort) -> Result<u16> {
         self.ports()
             .await?
             .map_to_host_port_ipv6(internal_port)

testcontainers/src/core/containers/request.rs

@@ -1,7 +1,7 @@
 use std::{borrow::Cow, collections::BTreeMap, net::IpAddr, time::Duration};
 
 use crate::{
-    core::{mounts::Mount, ports::ExposedPort, ContainerState, ExecCommand, WaitFor},
+    core::{mounts::Mount, ports::ContainerPort, ContainerState, ExecCommand, WaitFor},
     Image, TestcontainersError,
 };
 
@@ -26,11 +26,11 @@ pub struct ContainerRequest<I: Image> {
     pub(crate) startup_timeout: Option<Duration>,
 }
 
-/// Represents a port mapping between a local port and the internal port of a container.
+/// Represents a port mapping between a host's external port and the internal port of a container.
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub struct PortMapping {
-    pub local: u16,
-    pub internal: ExposedPort,
+    pub(crate) host_port: u16,
+    pub(crate) container_port: ContainerPort,
 }
 
 #[derive(parse_display::Display, Debug, Clone)]
@@ -129,7 +129,7 @@ impl<I: Image> ContainerRequest<I> {
         self.image.ready_conditions()
     }
 
-    pub fn expose_ports(&self) -> &[ExposedPort] {
+    pub fn expose_ports(&self) -> &[ContainerPort] {
         self.image.expose_ports()
     }
 
@@ -168,8 +168,19 @@ impl<I: Image> From<I> for ContainerRequest<I> {
     }
 }
 
-impl From<(u16, ExposedPort)> for PortMapping {
-    fn from((local, internal): (u16, ExposedPort)) -> Self {
-        PortMapping { local, internal }
+impl PortMapping {
+    pub(crate) fn new(local: u16, internal: ContainerPort) -> Self {
+        Self {
+            host_port: local,
+            container_port: internal,
+        }
+    }
+
+    pub fn host_port(&self) -> u16 {
+        self.host_port
+    }
+
+    pub fn container_port(&self) -> ContainerPort {
+        self.container_port
     }
 }

testcontainers/src/core/containers/sync_container.rs

@@ -1,7 +1,7 @@
 use std::{fmt, io::BufRead, net::IpAddr, sync::Arc};
 
 use crate::{
-    core::{env, error::Result, ports::Ports, ExecCommand, ExposedPort},
+    core::{env, error::Result, ports::Ports, ContainerPort, ExecCommand},
     ContainerAsync, Image,
 };
 
@@ -82,7 +82,7 @@ where
     ///
     /// This method does **not** magically expose the given port, it simply performs a mapping on
     /// the already exposed ports. If a docker container does not expose a port, this method returns an error.
-    pub fn get_host_port_ipv4(&self, internal_port: ExposedPort) -> Result<u16> {
+    pub fn get_host_port_ipv4(&self, internal_port: ContainerPort) -> Result<u16> {
         self.rt()
             .block_on(self.async_impl().get_host_port_ipv4(internal_port))
     }
@@ -92,7 +92,7 @@ where
     ///
     /// This method does **not** magically expose the given port, it simply performs a mapping on
     /// the already exposed ports. If a docker container does not expose a port, this method returns an error.
-    pub fn get_host_port_ipv6(&self, internal_port: ExposedPort) -> Result<u16> {
+    pub fn get_host_port_ipv6(&self, internal_port: ContainerPort) -> Result<u16> {
         self.rt()
             .block_on(self.async_impl().get_host_port_ipv6(internal_port))
     }

testcontainers/src/core/error.rs

@@ -1,7 +1,7 @@
 use std::error::Error;
 
 use crate::core::logs::WaitLogError;
-pub use crate::core::{client::ClientError, env::ConfigurationError, ExposedPort};
+pub use crate::core::{client::ClientError, env::ConfigurationError, ContainerPort};
 
 pub type Result<T> = std::result::Result<T, TestcontainersError>;
 
@@ -15,7 +15,7 @@ pub enum TestcontainersError {
     WaitContainer(#[from] WaitContainerError),
     /// Represents an error when a container does not expose a specified port
     #[error("container '{id}' does not expose port {port}")]
-    PortNotExposed { id: String, port: ExposedPort },
+    PortNotExposed { id: String, port: ContainerPort },
     /// Represents an error when a container is missing some information
     #[error(transparent)]
     MissingInfo(#[from] ContainerMissingInfo),

testcontainers/src/core/image.rs

@@ -4,7 +4,7 @@ pub use exec::{CmdWaitFor, ExecCommand};
 pub use image_ext::ImageExt;
 pub use wait_for::WaitFor;
 
-use super::ports::{ExposedPort, Ports};
+use super::ports::{ContainerPort, Ports};
 use crate::{core::mounts::Mount, TestcontainersError};
 
 mod exec;
@@ -69,7 +69,7 @@ where
     ///
     /// This method is useful when there is a need to expose some ports, but there is
     /// no `EXPOSE` instruction in the Dockerfile of an image.
-    fn expose_ports(&self) -> &[ExposedPort] {
+    fn expose_ports(&self) -> &[ContainerPort] {
         &[]
     }
 
@@ -104,10 +104,10 @@ impl ContainerState {
         }
     }
 
-    /// Returns the host port for the given internal port (`IPv4`).
+    /// Returns the host port for the given internal container's port (`IPv4`).
     ///
     /// Results in an error ([`TestcontainersError::PortNotExposed`]) if the port is not exposed.
-    pub fn host_port_ipv4(&self, internal_port: ExposedPort) -> Result<u16, TestcontainersError> {
+    pub fn host_port_ipv4(&self, internal_port: ContainerPort) -> Result<u16, TestcontainersError> {
         self.ports
             .map_to_host_port_ipv4(internal_port)
             .ok_or_else(|| TestcontainersError::PortNotExposed {
@@ -116,10 +116,10 @@ impl ContainerState {
             })
     }
 
-    /// Returns the host port for the given internal port (`IPv6`).
+    /// Returns the host port for the given internal container's port (`IPv6`).
     ///
     /// Results in an error ([`TestcontainersError::PortNotExposed`]) if the port is not exposed.
-    pub fn host_port_ipv6(&self, internal_port: ExposedPort) -> Result<u16, TestcontainersError> {
+    pub fn host_port_ipv6(&self, internal_port: ContainerPort) -> Result<u16, TestcontainersError> {
         self.ports
             .map_to_host_port_ipv6(internal_port)
             .ok_or_else(|| TestcontainersError::PortNotExposed {

testcontainers/src/core/image/image_ext.rs

@@ -1,7 +1,7 @@
 use std::time::Duration;
 
 use crate::{
-    core::{CgroupnsMode, Host, Mount, PortMapping},
+    core::{CgroupnsMode, ContainerPort, Host, Mount, PortMapping},
     ContainerRequest, Image,
 };
 
@@ -52,8 +52,17 @@ pub trait ImageExt<I: Image> {
     /// Adds a mount to the container.
     fn with_mount(self, mount: impl Into<Mount>) -> ContainerRequest<I>;
 
-    /// Adds a port mapping to the container.
-    fn with_mapped_port<P: Into<PortMapping>>(self, port: P) -> ContainerRequest<I>;
+    /// Adds a port mapping to the container, mapping the host port to the container's internal port.
+    ///
+    /// # Examples
+    /// ```rust,no_run
+    /// use testcontainers::{GenericImage, ImageExt};
+    /// use testcontainers::core::IntoContainerPort;
+    ///
+    /// let image = GenericImage::new("image", "tag").with_mapped_port(8080, 80.tcp());
+    /// ```
+    fn with_mapped_port(self, host_port: u16, container_port: ContainerPort)
+        -> ContainerRequest<I>;
 
     /// Sets the container to run in privileged mode.
     fn with_privileged(self, privileged: bool) -> ContainerRequest<I>;
@@ -139,10 +148,14 @@ impl<RI: Into<ContainerRequest<I>>, I: Image> ImageExt<I> for RI {
         runnable
     }
 
-    fn with_mapped_port<P: Into<PortMapping>>(self, port: P) -> ContainerRequest<I> {
+    fn with_mapped_port(
+        self,
+        host_port: u16,
+        container_port: ContainerPort,
+    ) -> ContainerRequest<I> {
         let runnable = self.into();
         let mut ports = runnable.ports.unwrap_or_default();
-        ports.push(port.into());
+        ports.push(PortMapping::new(host_port, container_port));
 
         ContainerRequest {
             ports: Some(ports),

testcontainers/src/core/ports.rs

@@ -5,7 +5,7 @@ use bollard_stubs::models::{PortBinding, PortMap};
 #[derive(
     parse_display::Display, parse_display::FromStr, Debug, Clone, Copy, Eq, PartialEq, Hash,
 )]
-pub enum ExposedPort {
+pub enum ContainerPort {
     #[display("{0}/tcp")]
     Tcp(u16),
     #[display("{0}/udp")]
@@ -14,17 +14,27 @@ pub enum ExposedPort {
     Sctp(u16),
 }
 
+/// A trait to allow easy conversion of a `u16` into a `ContainerPort`.
+/// For example, `123.tcp()` is equivalent to `ContainerPort::Tcp(123)`.
+pub trait IntoContainerPort {
+    fn tcp(self) -> ContainerPort;
+    fn udp(self) -> ContainerPort;
+    fn sctp(self) -> ContainerPort;
+}
+
 #[derive(Debug, thiserror::Error)]
 pub enum PortMappingError {
-    #[error("failed to parse port: {0}")]
-    FailedToParsePort(#[from] ParseIntError),
+    #[error("failed to parse container port: {0}")]
+    FailedToParseContainerPort(parse_display::ParseError),
+    #[error("failed to parse host port: {0}")]
+    FailedToParseHostPort(ParseIntError),
 }
 
 /// The exposed ports of a running container.
 #[derive(Clone, Debug, Default, Eq, PartialEq)]
 pub struct Ports {
-    ipv4_mapping: HashMap<ExposedPort, u16>,
-    ipv6_mapping: HashMap<ExposedPort, u16>,
+    ipv4_mapping: HashMap<ContainerPort, u16>,
+    ipv6_mapping: HashMap<ContainerPort, u16>,
 }
 
 impl Ports {
@@ -52,14 +62,14 @@ impl Ports {
         Self::try_from(port_binding)
     }
 
-    /// Returns the host port for the given internal port, on the host's IPv4 interfaces.
-    pub fn map_to_host_port_ipv4(&self, internal_port: ExposedPort) -> Option<u16> {
-        self.ipv4_mapping.get(&internal_port).cloned()
+    /// Returns the host port for the given internal container's port, on the host's IPv4 interfaces.
+    pub fn map_to_host_port_ipv4(&self, container_port: ContainerPort) -> Option<u16> {
+        self.ipv4_mapping.get(&container_port).cloned()
     }
 
-    /// Returns the host port for the given internal port, on the host's IPv6 interfaces.
-    pub fn map_to_host_port_ipv6(&self, internal_port: ExposedPort) -> Option<u16> {
-        self.ipv6_mapping.get(&internal_port).cloned()
+    /// Returns the host port for the given internal container's port, on the host's IPv6 interfaces.
+    pub fn map_to_host_port_ipv6(&self, container_port: ContainerPort) -> Option<u16> {
+        self.ipv6_mapping.get(&container_port).cloned()
     }
 }
 
@@ -71,35 +81,39 @@ impl TryFrom<PortMap> for Ports {
         let mut ipv6_mapping = HashMap::new();
         for (internal, external) in ports {
             // internal is of the form '8332/tcp', split off the protocol ...
-            let internal_port = internal.parse::<ExposedPort>().expect("Internal port");
+            let container_port = internal
+                .parse::<ContainerPort>()
+                .map_err(PortMappingError::FailedToParseContainerPort)?;
 
             // get the `HostPort` of each external port binding
             for binding in external.into_iter().flatten() {
-                if let Some(external_port) = binding.host_port.as_ref() {
-                    let external_port = external_port.parse()?;
+                if let Some(host_port) = binding.host_port.as_ref() {
+                    let host_port = host_port
+                        .parse()
+                        .map_err(PortMappingError::FailedToParseHostPort)?;
 
                     // switch on the IP version of the `HostIp`
                     let mapping = match binding.host_ip.map(|ip| ip.parse()) {
                         Some(Ok(IpAddr::V4(_))) => {
                             log::debug!(
                                 "Registering IPv4 port mapping: {} -> {}",
-                                internal_port,
-                                external_port
+                                container_port,
+                                host_port
                             );
                             &mut ipv4_mapping
                         }
                         Some(Ok(IpAddr::V6(_))) => {
                             log::debug!(
                                 "Registering IPv6 port mapping: {} -> {}",
-                                internal_port,
-                                external_port
+                                container_port,
+                                host_port
                             );
                             &mut ipv6_mapping
                         }
                         Some(Err(_)) | None => continue,
                     };
 
-                    mapping.insert(internal_port, external_port);
+                    mapping.insert(container_port, host_port);
                 } else {
                     continue;
                 }
@@ -113,6 +127,20 @@ impl TryFrom<PortMap> for Ports {
     }
 }
 
+impl IntoContainerPort for u16 {
+    fn tcp(self) -> ContainerPort {
+        ContainerPort::Tcp(self)
+    }
+
+    fn udp(self) -> ContainerPort {
+        ContainerPort::Udp(self)
+    }
+
+    fn sctp(self) -> ContainerPort {
+        ContainerPort::Sctp(self)
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use bollard_stubs::models::ContainerInspectResponse;
@@ -360,21 +388,11 @@ mod tests {
             .unwrap_or_default();
 
         let mut expected_ports = Ports::default();
-        expected_ports
-            .ipv6_mapping
-            .insert(ExposedPort::Tcp(8333), 49718);
-        expected_ports
-            .ipv4_mapping
-            .insert(ExposedPort::Sctp(8332), 33078);
-        expected_ports
-            .ipv4_mapping
-            .insert(ExposedPort::Tcp(18332), 33076);
-        expected_ports
-            .ipv4_mapping
-            .insert(ExposedPort::Tcp(8333), 33077);
-        expected_ports
-            .ipv4_mapping
-            .insert(ExposedPort::Udp(18333), 33075);
+        expected_ports.ipv6_mapping.insert(8333.tcp(), 49718);
+        expected_ports.ipv4_mapping.insert(8332.sctp(), 33078);
+        expected_ports.ipv4_mapping.insert(18332.tcp(), 33076);
+        expected_ports.ipv4_mapping.insert(8333.tcp(), 33077);
+        expected_ports.ipv4_mapping.insert(18333.udp(), 33075);
 
         assert_eq!(parsed_ports, expected_ports)
     }