From 1e0f7ec178bfb584869d08a6213521c240c7c443 Mon Sep 17 00:00:00 2001
From: Marek Franciszkiewicz <mfranciszkiewicz@users.noreply.github.com>
Date: Mon, 20 Jun 2022 20:10:20 +0200
Subject: [PATCH] Reinforce `Addresses` API (#29)

Makes sure that addresses are always sorted and de-duplicated
---
 crates/ya-http-proxy-model/src/addr.rs  | 71 +++++++++++++------------
 crates/ya-http-proxy-model/src/model.rs |  7 +--
 crates/ya-http-proxy/src/bin.rs         |  2 +-
 crates/ya-http-proxy/src/conf/server.rs |  5 +-
 4 files changed, 42 insertions(+), 43 deletions(-)

diff --git a/crates/ya-http-proxy-model/src/addr.rs b/crates/ya-http-proxy-model/src/addr.rs
index 8d11a7e..ddf537f 100644
--- a/crates/ya-http-proxy-model/src/addr.rs
+++ b/crates/ya-http-proxy-model/src/addr.rs
@@ -1,6 +1,7 @@
+use std::collections::HashSet;
 use std::fmt::{Display, Formatter};
 use std::net::SocketAddr;
-use std::ops::Add;
+use std::ops::{Add, AddAssign};
 
 use serde::{de, Deserialize, Serialize};
 
@@ -8,9 +9,23 @@ use crate::deser::one_or_many::OneOrManyVisitor;
 
 /// Socket address collection wrapper
 #[derive(Default, Clone, Debug, PartialEq, Eq, Hash, Serialize)]
-pub struct Addresses(pub Vec<SocketAddr>);
+pub struct Addresses(Vec<SocketAddr>);
 
 impl Addresses {
+    pub fn new(addrs: Vec<SocketAddr>) -> Self {
+        Addresses::default() + addrs
+    }
+
+    pub fn ports(&self) -> HashSet<u16> {
+        self.0.iter().map(|a| a.port()).collect()
+    }
+
+    #[inline]
+    pub fn is_empty(&self) -> bool {
+        self.0.is_empty()
+    }
+
+    #[inline]
     pub fn to_vec(&self) -> Vec<SocketAddr> {
         self.0.clone()
     }
@@ -21,28 +36,31 @@ impl<'de> Deserialize<'de> for Addresses {
     where
         D: de::Deserializer<'de>,
     {
-        let mut addrs: Vec<SocketAddr> =
-            deserializer.deserialize_any(OneOrManyVisitor::<SocketAddr>::default())?;
-
+        let addrs = Addresses::new(
+            deserializer.deserialize_any(OneOrManyVisitor::<SocketAddr>::default())?,
+        );
         if addrs.is_empty() {
             return Err(de::Error::custom("empty sequence"));
         }
-
-        addrs.sort();
-        addrs.dedup();
-
-        Ok(Addresses(addrs.into_iter().collect()))
+        Ok(addrs)
     }
 }
 
-impl Add for Addresses {
+impl<I: IntoIterator<Item = SocketAddr>> Add<I> for Addresses {
     type Output = Self;
 
-    fn add(mut self, rhs: Self) -> Self {
-        self.0.extend(rhs.0);
+    #[inline]
+    fn add(mut self, rhs: I) -> Self::Output {
+        self.add_assign(rhs);
+        self
+    }
+}
+
+impl<I: IntoIterator<Item = SocketAddr>> AddAssign<I> for Addresses {
+    fn add_assign(&mut self, rhs: I) {
+        self.0.extend(rhs);
         self.0.sort();
         self.0.dedup();
-        self
     }
 }
 
@@ -53,10 +71,12 @@ impl From<SocketAddr> for Addresses {
     }
 }
 
-impl From<Vec<SocketAddr>> for Addresses {
-    #[inline]
-    fn from(vec: Vec<SocketAddr>) -> Self {
-        Self(vec)
+impl IntoIterator for Addresses {
+    type Item = SocketAddr;
+    type IntoIter = std::vec::IntoIter<Self::Item>;
+
+    fn into_iter(self) -> Self::IntoIter {
+        self.0.into_iter()
     }
 }
 
@@ -72,18 +92,3 @@ impl Display for Addresses {
         write!(f, "]")
     }
 }
-
-impl Addresses {
-    pub fn gather_and_sort_addresses(
-        addresses1: Option<Addresses>,
-        addresses2: Option<Addresses>,
-    ) -> Addresses {
-        let mut addrs = addresses1.unwrap_or_default();
-        if let Some(addresses2) = addresses2 {
-            addrs.0.extend(addresses2.0);
-        }
-        addrs.0.sort();
-        addrs.0.dedup();
-        addrs
-    }
-}
diff --git a/crates/ya-http-proxy-model/src/model.rs b/crates/ya-http-proxy-model/src/model.rs
index 4137d06..238186d 100644
--- a/crates/ya-http-proxy-model/src/model.rs
+++ b/crates/ya-http-proxy-model/src/model.rs
@@ -121,10 +121,7 @@ pub struct CreateService {
 
 impl CreateService {
     pub fn addresses(&self) -> Addresses {
-        //make sure the function returns the same result as ServerConf::addresses
-        //otherwise services will be recreated resulting an error
-
-        Addresses::gather_and_sort_addresses(self.bind_http.clone(), self.bind_https.clone())
+        self.bind_https.clone().unwrap_or_default() + self.bind_http.clone().unwrap_or_default()
     }
 
     pub fn https_ports(&self) -> HashSet<u16> {
@@ -137,7 +134,7 @@ impl CreateService {
 
     fn ports(bind: &Option<Addresses>) -> HashSet<u16> {
         match bind {
-            Some(addrs) => addrs.0.iter().map(|a| a.port()).collect(),
+            Some(addrs) => addrs.ports(),
             None => Default::default(),
         }
     }
diff --git a/crates/ya-http-proxy/src/bin.rs b/crates/ya-http-proxy/src/bin.rs
index a452f3f..22040db 100644
--- a/crates/ya-http-proxy/src/bin.rs
+++ b/crates/ya-http-proxy/src/bin.rs
@@ -41,7 +41,7 @@ struct Cli {
 impl Cli {
     fn update_conf(&self, conf: &mut ProxyConf) {
         if let Some(addr) = self.default_addr {
-            conf.server.bind_https = Some(addr.into());
+            conf.server.bind_https.replace(addr.into());
         }
         if let Some(ref path) = self.default_cert {
             conf.server.server_cert.server_cert_store_path = Some(path.clone());
diff --git a/crates/ya-http-proxy/src/conf/server.rs b/crates/ya-http-proxy/src/conf/server.rs
index 1437413..cab9f08 100644
--- a/crates/ya-http-proxy/src/conf/server.rs
+++ b/crates/ya-http-proxy/src/conf/server.rs
@@ -62,10 +62,7 @@ pub struct ServerConf {
 
 impl ServerConf {
     pub fn addresses(&self) -> Addresses {
-        //make sure the function returns the same result as CreateService::addresses
-        //otherwise services will be recreated resulting an error
-
-        Addresses::gather_and_sort_addresses(self.bind_http.clone(), self.bind_https.clone())
+        self.bind_https.clone().unwrap_or_default() + self.bind_http.clone().unwrap_or_default()
     }
 }