From 989233a2cabbbca1670d684694dd66770c4115f5 Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Mon, 11 Nov 2024 07:44:43 +0000 Subject: [PATCH] refactor(net): remove inner from Net --- examples/get.rs | 2 +- src/net/system.rs | 259 +++++++++++++++------------------------------- tests/net.rs | 53 ++++++++-- 3 files changed, 128 insertions(+), 186 deletions(-) diff --git a/examples/get.rs b/examples/get.rs index 5d91e88..337b53b 100644 --- a/examples/get.rs +++ b/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(); } } diff --git a/src/net/system.rs b/src/net/system.rs index dc0a8a3..5bc6b78 100644 --- a/src/net/system.rs +++ b/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; @@ -48,24 +35,19 @@ struct Plan { } /// An abstraction for the network -#[derive(Debug)] pub struct Net { - inner: InnerNet, - mock: Option>, + plans: Option>, } impl Net { /// Creates a new unmocked [Net] for creating real network requests. pub(super) const fn new() -> Self { - Self { - inner: InnerNet::::new(), - mock: None, - } + Self { plans: None } } /// Creats a new [MockNet] for use in tests. pub(super) const fn mock() -> MockNet { MockNet { - inner: InnerNet::::new(), + plans: RefCell::new(vec![]), } } } @@ -107,17 +89,46 @@ impl Net { &self, request: impl Into, ) -> Result { - match &self.mock { - Some(mock) => mock.send(request).await, - None => self.inner.send(request).await, - } - } -} -impl Default for Net { - fn default() -> Self { - Self { - inner: InnerNet::::new(), - mock: None, + 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) + } + None => Err(Error::UnexpectedMockRequest(request)), } } } @@ -125,8 +136,10 @@ impl TryFrom for MockNet { type Error = super::Error; fn try_from(net: Net) -> std::result::Result { - 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 for MockNet { /// # Ok(()) /// # } /// ``` -#[derive(Debug)] pub struct MockNet { - inner: InnerNet, + plans: RefCell, } impl MockNet { /// Helper to create a default [reqwest::Client]. @@ -190,8 +202,29 @@ impl MockNet { /// # Ok(()) /// # } /// ``` - pub fn on(&self, request: impl Into) -> OnRequest { - self.inner.on(request) + pub fn on(&self, request_builder: impl Into) -> 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, + ) -> 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, - ) -> Result { - 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 for Net { fn from(mock_net: MockNet) -> Self { Self { - inner: InnerNet::::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 { - _type: PhantomData, - plans: RwLock, -} -impl InnerNet { - const fn new() -> Self { - Self { - _type: PhantomData, - plans: RwLock::new(vec![]), - } - } - - pub async fn send( - &self, - request: impl Into, - ) -> Result { - request.into().send().await.map_err(Error::from) - } -} - -impl InnerNet { - const fn new() -> Self { - Self { - _type: PhantomData, - plans: RwLock::new(vec![]), - } - } - - pub async fn send( - &self, - request: impl Into, - ) -> Result { - 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) -> 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, - ) -> 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 Drop for InnerNet { +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, + net: &'net MockNet, request: reqwest::Request, match_on: Vec, }, diff --git a/tests/net.rs b/tests/net.rs index 7e608f8..d297225 100644 --- a/tests/net.rs +++ b/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 +}