mirror of
https://github.com/TrueLayer/reqwest-middleware.git
synced 2024-12-26 10:56:32 +00:00
chore: revert change switching from Arc
to Box
in public APIs (#142)
fixes #139 There are other potential long term solutions listed [here](https://github.com/TrueLayer/reqwest-middleware/issues/139#issuecomment-2045946644) > I didn't put much thought into it. Internally there used to be a `Box<[Arc<dyn Middleware>]>` which is cloned about quite often. > > * https://docs.rs/reqwest-middleware/0.2.5/src/reqwest_middleware/client.rs.html#87 > * https://docs.rs/reqwest-middleware/0.2.5/src/reqwest_middleware/client.rs.html#139 > > Because of all the cloning, I decided to flip it to be `Arc<Box>` instead of `Box<Arc>`. > > I see three solutions here. > > 1. The one I'd lean towards is `Arc<Arc>` even if it's a bit silly, but prevents extra unnecessary allocations on each request. > 2. An alternative is to implement Middleware for Arc and then you can use the regular with(...) api. This would mean there's an extra box around your middleware > 3. Revert this particular change In the short term, I think it's best to go with option 3. This will unblock the next release. We can consider the other options for future releasees
This commit is contained in:
parent
0bdb2b6ef4
commit
e778b7df11
3 changed files with 26 additions and 25 deletions
|
@ -10,7 +10,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||||
- Updated `reqwest` to `0.12.0`.
|
- Updated `reqwest` to `0.12.0`.
|
||||||
- Removed `task_local_extensions` in favour of `http::Extensions`.
|
- Removed `task_local_extensions` in favour of `http::Extensions`.
|
||||||
- All extensions must be `Clone` now.
|
- All extensions must be `Clone` now.
|
||||||
- Replaced `with_arc` and `with_arc_init` methods with `with_box` and `with_box_init`.
|
|
||||||
- Support for `matchit` `0.8.0` - This means router matches like `/a/:some_var` need to be changed to `/a/{some_var}`
|
- Support for `matchit` `0.8.0` - This means router matches like `/a/:some_var` need to be changed to `/a/{some_var}`
|
||||||
|
|
||||||
### Changed
|
### Changed
|
||||||
|
|
|
@ -18,8 +18,8 @@ use crate::RequestInitialiser;
|
||||||
/// [`ClientWithMiddleware`]: crate::ClientWithMiddleware
|
/// [`ClientWithMiddleware`]: crate::ClientWithMiddleware
|
||||||
pub struct ClientBuilder {
|
pub struct ClientBuilder {
|
||||||
client: Client,
|
client: Client,
|
||||||
middleware_stack: Vec<Box<dyn Middleware>>,
|
middleware_stack: Vec<Arc<dyn Middleware>>,
|
||||||
initialiser_stack: Vec<Box<dyn RequestInitialiser>>,
|
initialiser_stack: Vec<Arc<dyn RequestInitialiser>>,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl ClientBuilder {
|
impl ClientBuilder {
|
||||||
|
@ -33,40 +33,40 @@ impl ClientBuilder {
|
||||||
|
|
||||||
/// Convenience method to attach middleware.
|
/// Convenience method to attach middleware.
|
||||||
///
|
///
|
||||||
/// If you need to keep a reference to the middleware after attaching, use [`with_box`].
|
/// If you need to keep a reference to the middleware after attaching, use [`with_arc`].
|
||||||
///
|
///
|
||||||
/// [`with_box`]: Self::with_box
|
/// [`with_arc`]: Self::with_arc
|
||||||
pub fn with<M>(self, middleware: M) -> Self
|
pub fn with<M>(self, middleware: M) -> Self
|
||||||
where
|
where
|
||||||
M: Middleware,
|
M: Middleware,
|
||||||
{
|
{
|
||||||
self.with_box(Box::new(middleware))
|
self.with_arc(Arc::new(middleware))
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Add middleware to the chain. [`with`] is more ergonomic if you don't need the `Box`.
|
/// Add middleware to the chain. [`with`] is more ergonomic if you don't need the `Arc`.
|
||||||
///
|
///
|
||||||
/// [`with`]: Self::with
|
/// [`with`]: Self::with
|
||||||
pub fn with_box(mut self, middleware: Box<dyn Middleware>) -> Self {
|
pub fn with_arc(mut self, middleware: Arc<dyn Middleware>) -> Self {
|
||||||
self.middleware_stack.push(middleware);
|
self.middleware_stack.push(middleware);
|
||||||
self
|
self
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Convenience method to attach a request initialiser.
|
/// Convenience method to attach a request initialiser.
|
||||||
///
|
///
|
||||||
/// If you need to keep a reference to the initialiser after attaching, use [`with_box_init`].
|
/// If you need to keep a reference to the initialiser after attaching, use [`with_arc_init`].
|
||||||
///
|
///
|
||||||
/// [`with_box_init`]: Self::with_box_init
|
/// [`with_arc_init`]: Self::with_arc_init
|
||||||
pub fn with_init<I>(self, initialiser: I) -> Self
|
pub fn with_init<I>(self, initialiser: I) -> Self
|
||||||
where
|
where
|
||||||
I: RequestInitialiser,
|
I: RequestInitialiser,
|
||||||
{
|
{
|
||||||
self.with_box_init(Box::new(initialiser))
|
self.with_arc_init(Arc::new(initialiser))
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Add a request initialiser to the chain. [`with_init`] is more ergonomic if you don't need the `Box`.
|
/// Add a request initialiser to the chain. [`with_init`] is more ergonomic if you don't need the `Arc`.
|
||||||
///
|
///
|
||||||
/// [`with_init`]: Self::with_init
|
/// [`with_init`]: Self::with_init
|
||||||
pub fn with_box_init(mut self, initialiser: Box<dyn RequestInitialiser>) -> Self {
|
pub fn with_arc_init(mut self, initialiser: Arc<dyn RequestInitialiser>) -> Self {
|
||||||
self.initialiser_stack.push(initialiser);
|
self.initialiser_stack.push(initialiser);
|
||||||
self
|
self
|
||||||
}
|
}
|
||||||
|
@ -75,8 +75,8 @@ impl ClientBuilder {
|
||||||
pub fn build(self) -> ClientWithMiddleware {
|
pub fn build(self) -> ClientWithMiddleware {
|
||||||
ClientWithMiddleware {
|
ClientWithMiddleware {
|
||||||
inner: self.client,
|
inner: self.client,
|
||||||
middleware_stack: self.middleware_stack.into(),
|
middleware_stack: self.middleware_stack.into_boxed_slice(),
|
||||||
initialiser_stack: self.initialiser_stack.into(),
|
initialiser_stack: self.initialiser_stack.into_boxed_slice(),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -86,21 +86,21 @@ impl ClientBuilder {
|
||||||
#[derive(Clone)]
|
#[derive(Clone)]
|
||||||
pub struct ClientWithMiddleware {
|
pub struct ClientWithMiddleware {
|
||||||
inner: reqwest::Client,
|
inner: reqwest::Client,
|
||||||
middleware_stack: Arc<[Box<dyn Middleware>]>,
|
middleware_stack: Box<[Arc<dyn Middleware>]>,
|
||||||
initialiser_stack: Arc<[Box<dyn RequestInitialiser>]>,
|
initialiser_stack: Box<[Arc<dyn RequestInitialiser>]>,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl ClientWithMiddleware {
|
impl ClientWithMiddleware {
|
||||||
/// See [`ClientBuilder`] for a more ergonomic way to build `ClientWithMiddleware` instances.
|
/// See [`ClientBuilder`] for a more ergonomic way to build `ClientWithMiddleware` instances.
|
||||||
pub fn new<T>(client: Client, middleware_stack: T) -> Self
|
pub fn new<T>(client: Client, middleware_stack: T) -> Self
|
||||||
where
|
where
|
||||||
T: Into<Arc<[Box<dyn Middleware>]>>,
|
T: Into<Box<[Arc<dyn Middleware>]>>,
|
||||||
{
|
{
|
||||||
ClientWithMiddleware {
|
ClientWithMiddleware {
|
||||||
inner: client,
|
inner: client,
|
||||||
middleware_stack: middleware_stack.into(),
|
middleware_stack: middleware_stack.into(),
|
||||||
// TODO(conradludgate) - allow downstream code to control this manually if desired
|
// TODO(conradludgate) - allow downstream code to control this manually if desired
|
||||||
initialiser_stack: Arc::from(vec![]),
|
initialiser_stack: Box::new([]),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -222,8 +222,8 @@ impl From<Client> for ClientWithMiddleware {
|
||||||
fn from(client: Client) -> Self {
|
fn from(client: Client) -> Self {
|
||||||
ClientWithMiddleware {
|
ClientWithMiddleware {
|
||||||
inner: client,
|
inner: client,
|
||||||
middleware_stack: Arc::from(vec![]),
|
middleware_stack: Box::new([]),
|
||||||
initialiser_stack: Arc::from(vec![]),
|
initialiser_stack: Box::new([]),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -314,8 +314,8 @@ mod service {
|
||||||
#[must_use = "RequestBuilder does nothing until you 'send' it"]
|
#[must_use = "RequestBuilder does nothing until you 'send' it"]
|
||||||
pub struct RequestBuilder {
|
pub struct RequestBuilder {
|
||||||
inner: reqwest::RequestBuilder,
|
inner: reqwest::RequestBuilder,
|
||||||
middleware_stack: Arc<[Box<dyn Middleware>]>,
|
middleware_stack: Box<[Arc<dyn Middleware>]>,
|
||||||
initialiser_stack: Arc<[Box<dyn RequestInitialiser>]>,
|
initialiser_stack: Box<[Arc<dyn RequestInitialiser>]>,
|
||||||
extensions: Extensions,
|
extensions: Extensions,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -3,6 +3,8 @@ use reqwest::{Client, Request, Response};
|
||||||
|
|
||||||
use crate::error::{Error, Result};
|
use crate::error::{Error, Result};
|
||||||
|
|
||||||
|
use std::sync::Arc;
|
||||||
|
|
||||||
/// When attached to a [`ClientWithMiddleware`] (generally using [`with`]), middleware is run
|
/// When attached to a [`ClientWithMiddleware`] (generally using [`with`]), middleware is run
|
||||||
/// whenever the client issues a request, in the order it was attached.
|
/// whenever the client issues a request, in the order it was attached.
|
||||||
///
|
///
|
||||||
|
@ -73,7 +75,7 @@ where
|
||||||
#[derive(Clone)]
|
#[derive(Clone)]
|
||||||
pub struct Next<'a> {
|
pub struct Next<'a> {
|
||||||
client: &'a Client,
|
client: &'a Client,
|
||||||
middlewares: &'a [Box<dyn Middleware>],
|
middlewares: &'a [Arc<dyn Middleware>],
|
||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(not(target_arch = "wasm32"))]
|
#[cfg(not(target_arch = "wasm32"))]
|
||||||
|
@ -82,7 +84,7 @@ pub type BoxFuture<'a, T> = std::pin::Pin<Box<dyn std::future::Future<Output = T
|
||||||
pub type BoxFuture<'a, T> = std::pin::Pin<Box<dyn std::future::Future<Output = T> + 'a>>;
|
pub type BoxFuture<'a, T> = std::pin::Pin<Box<dyn std::future::Future<Output = T> + 'a>>;
|
||||||
|
|
||||||
impl<'a> Next<'a> {
|
impl<'a> Next<'a> {
|
||||||
pub(crate) fn new(client: &'a Client, middlewares: &'a [Box<dyn Middleware>]) -> Self {
|
pub(crate) fn new(client: &'a Client, middlewares: &'a [Arc<dyn Middleware>]) -> Self {
|
||||||
Next {
|
Next {
|
||||||
client,
|
client,
|
||||||
middlewares,
|
middlewares,
|
||||||
|
|
Loading…
Reference in a new issue