refactor(net): remove inner from Net

examples/get.rs

@@ -147,6 +147,6 @@ mod tests {
         // not needed for this test, but should it be needed, we can avoid checking for any
         // unconsumed request matches.
         let mock_net = kxio::net::MockNet::try_from(net).expect("recover mock");
-        mock_net.reset().expect("reset mock");
+        mock_net.reset();
     }
 }

src/net/system.rs

@@ -1,23 +1,10 @@
 //
-use std::{marker::PhantomData, sync::RwLock};
+use std::cell::RefCell;
 
 use reqwest::Client;
 
 use super::{Error, Result};
 
-/// Marker trait used to identify whether a [Net] is mocked or not.
-pub trait NetType {}
-
-/// Marker struct that indicates that a [Net] is a mock.
-#[derive(Debug)]
-pub struct Mocked;
-impl NetType for Mocked {}
-
-/// Market struct that indicates that a [Net] is not mocked.
-#[derive(Debug)]
-pub struct Unmocked;
-impl NetType for Unmocked {}
-
 /// A list of planned requests and responses
 type Plans = Vec<Plan>;
 
@@ -48,24 +35,19 @@ struct Plan {
 }
 
 /// An abstraction for the network
-#[derive(Debug)]
 pub struct Net {
-    inner: InnerNet<Unmocked>,
-    mock: Option<InnerNet<Mocked>>,
+    plans: Option<RefCell<Plans>>,
 }
 impl Net {
     /// Creates a new unmocked [Net] for creating real network requests.
     pub(super) const fn new() -> Self {
-        Self {
-            inner: InnerNet::<Unmocked>::new(),
-            mock: None,
-        }
+        Self { plans: None }
     }
 
     /// Creats a new [MockNet] for use in tests.
     pub(super) const fn mock() -> MockNet {
         MockNet {
-            inner: InnerNet::<Mocked>::new(),
+            plans: RefCell::new(vec![]),
         }
     }
 }
@@ -107,17 +89,46 @@ impl Net {
         &self,
         request: impl Into<reqwest::RequestBuilder>,
     ) -> Result<reqwest::Response> {
-        match &self.mock {
-            Some(mock) => mock.send(request).await,
-            None => self.inner.send(request).await,
+        let Some(plans) = &self.plans else {
+            return request.into().send().await.map_err(Error::from);
+        };
+        let request = request.into().build()?;
+        let index = plans.borrow().iter().position(|plan| {
+            // METHOD
+            (if plan.match_on.contains(&MatchOn::Method) {
+                plan.request.method() == request.method()
+            } else {
+                true
+            })
+            // URL
+            && (if plan.match_on.contains(&MatchOn::Url) {
+                plan.request.url() == request.url()
+            } else {
+                true
+            })
+            // BODY
+            && (if plan.match_on.contains(&MatchOn::Body) {
+                match (plan.request.body(), request.body()) {
+                    (None, None) => true,
+                    (Some(plan), Some(req)) => plan.as_bytes().eq(&req.as_bytes()),
+                    _ => false,
                 }
+            } else {
+                true
+            })
+            // HEADERS
+            && (if plan.match_on.contains(&MatchOn::Headers) {
+                plan.request.headers() == request.headers()
+            } else {
+                true
+            })
+        });
+        match index {
+            Some(i) => {
+                let response = plans.borrow_mut().remove(i).response;
+                Ok(response)
             }
-}
-impl Default for Net {
-    fn default() -> Self {
-        Self {
-            inner: InnerNet::<Unmocked>::new(),
-            mock: None,
+            None => Err(Error::UnexpectedMockRequest(request)),
         }
     }
 }
@@ -125,8 +136,10 @@ impl TryFrom<Net> for MockNet {
     type Error = super::Error;
 
     fn try_from(net: Net) -> std::result::Result<Self, Self::Error> {
-        match net.mock {
-            Some(inner_mock) => Ok(MockNet { inner: inner_mock }),
+        match &net.plans {
+            Some(plans) => Ok(MockNet {
+                plans: RefCell::new(plans.take()),
+            }),
             None => Err(Self::Error::NetIsNotAMock),
         }
     }
@@ -158,9 +171,8 @@ impl TryFrom<Net> for MockNet {
 /// # Ok(())
 /// # }
 /// ```
-#[derive(Debug)]
 pub struct MockNet {
-    inner: InnerNet<Mocked>,
+    plans: RefCell<Plans>,
 }
 impl MockNet {
     /// Helper to create a default [reqwest::Client].
@@ -190,8 +202,29 @@ impl MockNet {
     /// # Ok(())
     /// # }
     /// ```
-    pub fn on(&self, request: impl Into<reqwest::RequestBuilder>) -> OnRequest {
-        self.inner.on(request)
+    pub fn on(&self, request_builder: impl Into<reqwest::RequestBuilder>) -> OnRequest {
+        match request_builder.into().build() {
+            Ok(request) => OnRequest::Valid {
+                net: self,
+                request,
+                match_on: vec![MatchOn::Method, MatchOn::Url],
+            },
+            Err(err) => OnRequest::Error(err.into()),
+        }
+    }
+
+    fn _on(
+        &self,
+        request: reqwest::Request,
+        response: reqwest::Response,
+        match_on: Vec<MatchOn>,
+    ) -> Result<()> {
+        self.plans.borrow_mut().push(Plan {
+            request,
+            response,
+            match_on,
+        });
+        Ok(())
     }
 
     /// Creates a [http::response::Builder] to be extended and returned by a mocked network request.
@@ -199,21 +232,6 @@ impl MockNet {
         Default::default()
     }
 
-    /// Constructs the request and matches it against the expected requests.
-    ///
-    /// The first on it finds where it matches against its criteria will be taken ("consumed") and
-    /// its response returned.
-    ///
-    /// The order the expected requests are scanned is the order they were declared. As each
-    /// expected request is successfully matched against, it is removed and can't be matched
-    /// against again.
-    pub async fn send(
-        &self,
-        request: impl Into<reqwest::RequestBuilder>,
-    ) -> Result<reqwest::Response> {
-        self.inner.send(request).await
-    }
-
     /// Clears all the expected requests and responses from the [MockNet].
     ///
     /// When the [MockNet] goes out of scope it will assert that all expected requests and
@@ -231,144 +249,37 @@ impl MockNet {
     /// # Ok(())
     /// # }
     /// ```
-    pub fn reset(&self) -> Result<()> {
-        self.inner.reset()
+    pub fn reset(&self) {
+        self.plans.take();
     }
 }
 impl From<MockNet> for Net {
     fn from(mock_net: MockNet) -> Self {
         Self {
-            inner: InnerNet::<Unmocked>::new(),
             // keep the original `inner` around to allow it's Drop impelmentation to run when we go
             // out of scope at the end of the test
-            mock: Some(mock_net.inner),
+            plans: Some(RefCell::new(mock_net.plans.take())),
         }
     }
 }
 
-/// Part of the inner workings of [Net] and [MockNet].
-///
-/// Holds the list of expected requests for [MockNet].
-#[derive(Debug)]
-pub struct InnerNet<T: NetType> {
-    _type: PhantomData<T>,
-    plans: RwLock<Plans>,
-}
-impl InnerNet<Unmocked> {
-    const fn new() -> Self {
-        Self {
-            _type: PhantomData,
-            plans: RwLock::new(vec![]),
-        }
-    }
-
-    pub async fn send(
-        &self,
-        request: impl Into<reqwest::RequestBuilder>,
-    ) -> Result<reqwest::Response> {
-        request.into().send().await.map_err(Error::from)
-    }
-}
-
-impl InnerNet<Mocked> {
-    const fn new() -> Self {
-        Self {
-            _type: PhantomData,
-            plans: RwLock::new(vec![]),
-        }
-    }
-
-    pub async fn send(
-        &self,
-        request: impl Into<reqwest::RequestBuilder>,
-    ) -> Result<reqwest::Response> {
-        let request = request.into().build()?;
-        let read_plans = self.plans.read().map_err(|_| Error::RwLockLocked)?;
-        let index = read_plans.iter().position(|plan| {
-            // METHOD
-            (if plan.match_on.contains(&MatchOn::Method) {
-                plan.request.method() == request.method()
-            } else {
-                true
-            })
-            // URL
-            && (if plan.match_on.contains(&MatchOn::Url) {
-                plan.request.url() == request.url()
-            } else {
-                true
-            })
-            // BODY
-            && (if plan.match_on.contains(&MatchOn::Body) {
-                match (plan.request.body(), request.body()) {
-                    (None, None) => true,
-                    (Some(plan), Some(req)) => plan.as_bytes().eq(&req.as_bytes()),
-                    _ => false,
-                }
-            } else {
-                true
-            })
-            // HEADERS
-            && (if plan.match_on.contains(&MatchOn::Headers) {
-                plan.request.headers() == request.headers()
-            } else {
-                true
-            })
-        });
-        drop(read_plans);
-        match index {
-            Some(i) => {
-                let mut write_plans = self.plans.write().map_err(|_| Error::RwLockLocked)?;
-                Ok(write_plans.remove(i).response)
-            }
-            None => Err(Error::UnexpectedMockRequest(request)),
-        }
-    }
-
-    pub fn on(&self, request_builder: impl Into<reqwest::RequestBuilder>) -> OnRequest {
-        match request_builder.into().build() {
-            Ok(request) => OnRequest::Valid {
-                net: self,
-                request,
-                match_on: vec![MatchOn::Method, MatchOn::Url],
-            },
-            Err(err) => OnRequest::Error(err.into()),
-        }
-    }
-
-    fn _on(
-        &self,
-        request: reqwest::Request,
-        response: reqwest::Response,
-        match_on: Vec<MatchOn>,
-    ) -> Result<()> {
-        let mut write_plans = self.plans.write().map_err(|_| Error::RwLockLocked)?;
-        write_plans.push(Plan {
-            request,
-            response,
-            match_on,
-        });
-        Ok(())
-    }
-
-    pub fn reset(&self) -> Result<()> {
-        let mut write_plans = self.plans.write().map_err(|_| Error::RwLockLocked)?;
-        write_plans.clear();
-        Ok(())
-    }
-}
-impl<T: NetType> Drop for InnerNet<T> {
+impl Drop for MockNet {
     fn drop(&mut self) {
-        let Ok(read_plans) = self.plans.read() else {
-            return;
-        };
-        assert!(read_plans.is_empty())
+        assert!(self.plans.borrow().is_empty())
+    }
+}
+impl Drop for Net {
+    fn drop(&mut self) {
+        if let Some(plans) = &self.plans {
+            assert!(plans.borrow().is_empty())
+        }
     }
 }
 
 /// Intermediate struct used while declaring an expected request and its response.
 pub enum OnRequest<'net> {
     Valid {
-        net: &'net InnerNet<Mocked>,
+        net: &'net MockNet,
         request: reqwest::Request,
         match_on: Vec<MatchOn>,
     },

tests/net.rs

@@ -1,6 +1,6 @@
 use assert2::let_assert;
 //
-use kxio::net::{Error, MatchOn, Net};
+use kxio::net::{Error, MatchOn, MockNet, Net};
 
 #[tokio::test]
 async fn test_get_url() {
@@ -30,17 +30,20 @@ async fn test_get_url() {
 #[tokio::test]
 async fn test_get_wrong_url() {
     //given
-    let net = kxio::net::mock();
-    let client = net.client();
+    let mock_net = kxio::net::mock();
+    let client = mock_net.client();
 
     let url = "https://www.example.com";
     let request = client.get(url);
-    let my_response = net.response().status(200).body("Get OK");
+    let my_response = mock_net.response().status(200).body("Get OK");
 
-    net.on(request)
+    mock_net
+        .on(request)
         .respond_with_body(my_response)
         .expect("on request, respond");
 
+    let net = Net::from(mock_net);
+
     //when
     let_assert!(
         Err(Error::UnexpectedMockRequest(invalid_request)) =
@@ -51,7 +54,8 @@ async fn test_get_wrong_url() {
     assert_eq!(invalid_request.url().to_string(), "https://some.other.url/");
 
     // remove pending unmatched request - we never meant to match against it
-    net.reset().expect("reset");
+    let mock_net = MockNet::try_from(net).expect("recover net");
+    mock_net.reset();
 }
 
 #[tokio::test]
@@ -214,19 +218,22 @@ async fn test_post_by_headers() {
 
 #[tokio::test]
 #[should_panic]
-async fn test_unused_post() {
+async fn test_unused_post_as_net() {
     //given
-    let net = kxio::net::mock();
-    let client = net.client();
+    let mock_net = kxio::net::mock();
+    let client = mock_net.client();
 
     let url = "https://www.example.com";
     let request = client.post(url);
-    let my_response = net.response().status(200).body("Post OK");
+    let my_response = mock_net.response().status(200).body("Post OK");
 
-    net.on(request)
+    mock_net
+        .on(request)
         .respond_with_body(my_response)
         .expect("on request, respond");
 
+    let _net = Net::from(mock_net);
+
     //when
     // don't send the planned request
     // let _response = Net::from(net).send(client.post(url)).await.expect("send");
@@ -234,3 +241,27 @@ async fn test_unused_post() {
     //then
     // Drop implementation for net should panic
 }
+
+#[tokio::test]
+#[should_panic]
+async fn test_unused_post_as_mocknet() {
+    //given
+    let mock_net = kxio::net::mock();
+    let client = mock_net.client();
+
+    let url = "https://www.example.com";
+    let request = client.post(url);
+    let my_response = mock_net.response().status(200).body("Post OK");
+
+    mock_net
+        .on(request)
+        .respond_with_body(my_response)
+        .expect("on request, respond");
+
+    //when
+    // don't send the planned request
+    // let _response = Net::from(net).send(client.post(url)).await.expect("send");
+
+    //then
+    // Drop implementation for mock_net should panic
+}