refactor(swarm): simplify DialOpts (#3335)

Instead of tracking an inner enum, move all fields of `Opts` into `DialOpts`, setting them directly once we construct them. This makes the getters a lot simpler to implement and reduces code duplication.

Author: thomaseizinger

swarm/src/dial_opts.rs

@@ -36,7 +36,14 @@ use std::num::NonZeroU8;
 ///
 /// - [`DialOpts::unknown_peer_id`] dialing an unknown peer
 #[derive(Debug)]
-pub struct DialOpts(pub(super) Opts);
+pub struct DialOpts {
+    peer_id: Option<PeerId>,
+    condition: PeerCondition,
+    addresses: Vec<Multiaddr>,
+    extend_addresses_through_behaviour: bool,
+    role_override: Endpoint,
+    dial_concurrency_factor_override: Option<NonZeroU8>,
+}
 
 impl DialOpts {
     /// Dial a known peer.
@@ -73,13 +80,7 @@ impl DialOpts {
 
     /// Get the [`PeerId`] specified in a [`DialOpts`] if any.
     pub fn get_peer_id(&self) -> Option<PeerId> {
-        match self {
-            DialOpts(Opts::WithPeerId(WithPeerId { peer_id, .. })) => Some(*peer_id),
-            DialOpts(Opts::WithPeerIdWithAddresses(WithPeerIdWithAddresses {
-                peer_id, ..
-            })) => Some(*peer_id),
-            DialOpts(Opts::WithoutPeerIdWithAddress(_)) => None,
-        }
+        self.peer_id
     }
 
     /// Retrieves the [`PeerId`] from the [`DialOpts`] if specified or otherwise tries to parse it
@@ -92,92 +93,48 @@ impl DialOpts {
     ///
     /// See <https://github.com/multiformats/rust-multiaddr/issues/73>.
     pub(crate) fn get_or_parse_peer_id(&self) -> Result<Option<PeerId>, Multihash> {
-        match self {
-            DialOpts(Opts::WithPeerId(WithPeerId { peer_id, .. })) => Ok(Some(*peer_id)),
-            DialOpts(Opts::WithPeerIdWithAddresses(WithPeerIdWithAddresses {
-                peer_id, ..
-            })) => Ok(Some(*peer_id)),
-            DialOpts(Opts::WithoutPeerIdWithAddress(WithoutPeerIdWithAddress {
-                address, ..
-            })) => {
-                let peer_id = address
-                    .iter()
-                    .last()
-                    .and_then(|p| {
-                        if let Protocol::P2p(ma) = p {
-                            Some(PeerId::try_from(ma))
-                        } else {
-                            None
-                        }
-                    })
-                    .transpose()?;
-
-                Ok(peer_id)
-            }
+        if let Some(peer_id) = self.peer_id {
+            return Ok(Some(peer_id));
         }
+
+        let first_address = match self.addresses.first() {
+            Some(first_address) => first_address,
+            None => return Ok(None),
+        };
+
+        let maybe_peer_id = first_address
+            .iter()
+            .last()
+            .and_then(|p| {
+                if let Protocol::P2p(ma) = p {
+                    Some(PeerId::try_from(ma))
+                } else {
+                    None
+                }
+            })
+            .transpose()?;
+
+        Ok(maybe_peer_id)
     }
 
     pub(crate) fn get_addresses(&self) -> Vec<Multiaddr> {
-        match self {
-            DialOpts(Opts::WithPeerId(WithPeerId { .. })) => vec![],
-            DialOpts(Opts::WithPeerIdWithAddresses(WithPeerIdWithAddresses {
-                addresses, ..
-            })) => addresses.clone(),
-            DialOpts(Opts::WithoutPeerIdWithAddress(WithoutPeerIdWithAddress {
-                address, ..
-            })) => vec![address.clone()],
-        }
+        self.addresses.clone()
     }
 
     pub(crate) fn extend_addresses_through_behaviour(&self) -> bool {
-        match self {
-            DialOpts(Opts::WithPeerId(WithPeerId { .. })) => true,
-            DialOpts(Opts::WithPeerIdWithAddresses(WithPeerIdWithAddresses {
-                extend_addresses_through_behaviour,
-                ..
-            })) => *extend_addresses_through_behaviour,
-            DialOpts(Opts::WithoutPeerIdWithAddress(WithoutPeerIdWithAddress { .. })) => true,
-        }
+        self.extend_addresses_through_behaviour
     }
 
     pub(crate) fn peer_condition(&self) -> PeerCondition {
-        match self {
-            DialOpts(
-                Opts::WithPeerId(WithPeerId { condition, .. })
-                | Opts::WithPeerIdWithAddresses(WithPeerIdWithAddresses { condition, .. }),
-            ) => *condition,
-            DialOpts(Opts::WithoutPeerIdWithAddress(WithoutPeerIdWithAddress { .. })) => {
-                PeerCondition::Always
-            }
-        }
+        self.condition
     }
 
     pub(crate) fn dial_concurrency_override(&self) -> Option<NonZeroU8> {
-        match self {
-            DialOpts(Opts::WithPeerId(WithPeerId {
-                dial_concurrency_factor_override,
-                ..
-            })) => *dial_concurrency_factor_override,
-            DialOpts(Opts::WithPeerIdWithAddresses(WithPeerIdWithAddresses {
-                dial_concurrency_factor_override,
-                ..
-            })) => *dial_concurrency_factor_override,
-            DialOpts(Opts::WithoutPeerIdWithAddress(WithoutPeerIdWithAddress { .. })) => None,
-        }
+        self.dial_concurrency_factor_override
     }
 
     pub(crate) fn role_override(&self) -> Endpoint {
-        match self {
-            DialOpts(Opts::WithPeerId(WithPeerId { role_override, .. })) => *role_override,
-            DialOpts(Opts::WithPeerIdWithAddresses(WithPeerIdWithAddresses {
-                role_override,
-                ..
-            })) => *role_override,
-            DialOpts(Opts::WithoutPeerIdWithAddress(WithoutPeerIdWithAddress {
-                role_override,
-                ..
-            })) => *role_override,
-        }
+        self.role_override
     }
 }
 
@@ -193,25 +150,12 @@ impl From<PeerId> for DialOpts {
     }
 }
 
-/// Internal options type.
-///
-/// Not to be constructed manually. Use either of the below instead:
-///
-/// - [`DialOpts::peer_id`] dialing a known peer
-/// - [`DialOpts::unknown_peer_id`] dialing an unknown peer
-#[derive(Debug)]
-pub(super) enum Opts {
-    WithPeerId(WithPeerId),
-    WithPeerIdWithAddresses(WithPeerIdWithAddresses),
-    WithoutPeerIdWithAddress(WithoutPeerIdWithAddress),
-}
-
 #[derive(Debug)]
 pub struct WithPeerId {
-    pub(crate) peer_id: PeerId,
-    pub(crate) condition: PeerCondition,
-    pub(crate) role_override: Endpoint,
-    pub(crate) dial_concurrency_factor_override: Option<NonZeroU8>,
+    peer_id: PeerId,
+    condition: PeerCondition,
+    role_override: Endpoint,
+    dial_concurrency_factor_override: Option<NonZeroU8>,
 }
 
 impl WithPeerId {
@@ -256,18 +200,25 @@ impl WithPeerId {
     /// Addresses to dial the peer are retrieved via
     /// [`NetworkBehaviour::addresses_of_peer`](crate::behaviour::NetworkBehaviour::addresses_of_peer).
     pub fn build(self) -> DialOpts {
-        DialOpts(Opts::WithPeerId(self))
+        DialOpts {
+            peer_id: Some(self.peer_id),
+            condition: self.condition,
+            addresses: vec![],
+            extend_addresses_through_behaviour: true,
+            role_override: self.role_override,
+            dial_concurrency_factor_override: self.dial_concurrency_factor_override,
+        }
     }
 }
 
 #[derive(Debug)]
 pub struct WithPeerIdWithAddresses {
-    pub(crate) peer_id: PeerId,
-    pub(crate) condition: PeerCondition,
-    pub(crate) addresses: Vec<Multiaddr>,
-    pub(crate) extend_addresses_through_behaviour: bool,
-    pub(crate) role_override: Endpoint,
-    pub(crate) dial_concurrency_factor_override: Option<NonZeroU8>,
+    peer_id: PeerId,
+    condition: PeerCondition,
+    addresses: Vec<Multiaddr>,
+    extend_addresses_through_behaviour: bool,
+    role_override: Endpoint,
+    dial_concurrency_factor_override: Option<NonZeroU8>,
 }
 
 impl WithPeerIdWithAddresses {
@@ -304,7 +255,14 @@ impl WithPeerIdWithAddresses {
 
     /// Build the final [`DialOpts`].
     pub fn build(self) -> DialOpts {
-        DialOpts(Opts::WithPeerIdWithAddresses(self))
+        DialOpts {
+            peer_id: Some(self.peer_id),
+            condition: self.condition,
+            addresses: self.addresses,
+            extend_addresses_through_behaviour: self.extend_addresses_through_behaviour,
+            role_override: self.role_override,
+            dial_concurrency_factor_override: self.dial_concurrency_factor_override,
+        }
     }
 }
 
@@ -323,8 +281,8 @@ impl WithoutPeerId {
 
 #[derive(Debug)]
 pub struct WithoutPeerIdWithAddress {
-    pub(crate) address: Multiaddr,
-    pub(crate) role_override: Endpoint,
+    address: Multiaddr,
+    role_override: Endpoint,
 }
 
 impl WithoutPeerIdWithAddress {
@@ -340,7 +298,14 @@ impl WithoutPeerIdWithAddress {
     }
     /// Build the final [`DialOpts`].
     pub fn build(self) -> DialOpts {
-        DialOpts(Opts::WithoutPeerIdWithAddress(self))
+        DialOpts {
+            peer_id: None,
+            condition: PeerCondition::Always,
+            addresses: vec![self.address],
+            extend_addresses_through_behaviour: false,
+            role_override: self.role_override,
+            dial_concurrency_factor_override: None,
+        }
     }
 }