Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

extensions middleware and request extensions #56

Merged
merged 7 commits into from
Sep 30, 2022
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Added
- Implementation of `Debug` trait for `RequestBuilder`.
- A new `RequestInitialiser` trait that can be added to `ClientWithMiddleware`
- A new `Extension` initialiser that adds extensions to the request
- Adds `with_extension` method functionality to `RequestBuilder` that can add extensions for the `send` method to use - deprecating `send_with_extensions`.

## [0.1.6] - 2022-04-21

Absolutely nothing changed

## [0.1.5] - 2022-02-21

Expand Down
100 changes: 79 additions & 21 deletions reqwest-middleware/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,23 @@ use task_local_extensions::Extensions;

use crate::error::Result;
use crate::middleware::{Middleware, Next};
use crate::RequestInitialiser;

/// A `ClientBuilder` is used to build a [`ClientWithMiddleware`].
///
/// [`ClientWithMiddleware`]: crate::ClientWithMiddleware
pub struct ClientBuilder {
client: Client,
middleware_stack: Vec<Arc<dyn Middleware>>,
initialiser_stack: Vec<Arc<dyn RequestInitialiser>>,
}

impl ClientBuilder {
pub fn new(client: Client) -> Self {
ClientBuilder {
client,
middleware_stack: Vec::new(),
initialiser_stack: Vec::new(),
}
}

Expand All @@ -47,9 +50,33 @@ impl ClientBuilder {
self
}

/// Convenience method to attach middleware.
conradludgate marked this conversation as resolved.
Show resolved Hide resolved
///
/// If you need to keep a reference to the middleware after attaching, use [`with_arc`].
///
/// [`with_arc`]: Self::with_arc
conradludgate marked this conversation as resolved.
Show resolved Hide resolved
pub fn with_init<I>(self, initialiser: I) -> Self
where
I: RequestInitialiser,
{
self.with_arc_init(Arc::new(initialiser))
}

/// Add initialiser to the chain. [`with`] is more ergonomic if you don't need the `Arc`.
///
/// [`with`]: Self::with
conradludgate marked this conversation as resolved.
Show resolved Hide resolved
pub fn with_arc_init(mut self, initialiser: Arc<dyn RequestInitialiser>) -> Self {
self.initialiser_stack.push(initialiser);
self
}

/// Returns a `ClientWithMiddleware` using this builder configuration.
pub fn build(self) -> ClientWithMiddleware {
ClientWithMiddleware::new(self.client, self.middleware_stack)
ClientWithMiddleware {
inner: self.client,
middleware_stack: self.middleware_stack.into_boxed_slice(),
initialiser_stack: self.initialiser_stack.into_boxed_slice(),
}
}
}

Expand All @@ -59,6 +86,7 @@ impl ClientBuilder {
pub struct ClientWithMiddleware {
inner: reqwest::Client,
middleware_stack: Box<[Arc<dyn Middleware>]>,
initialiser_stack: Box<[Arc<dyn RequestInitialiser>]>,
}

impl ClientWithMiddleware {
Expand All @@ -70,6 +98,8 @@ impl ClientWithMiddleware {
ClientWithMiddleware {
inner: client,
middleware_stack: middleware_stack.into(),
// TODO(conradludgate) - allow downstream code to control this manually if desired
initialiser_stack: Box::new([]),
}
}

Expand Down Expand Up @@ -105,10 +135,14 @@ impl ClientWithMiddleware {

/// See [`Client::request`]
pub fn request<U: IntoUrl>(&self, method: Method, url: U) -> RequestBuilder {
RequestBuilder {
let req = RequestBuilder {
inner: self.inner.request(method, url),
client: self.clone(),
}
extensions: Extensions::new(),
};
self.initialiser_stack
.iter()
.fold(req, |req, i| i.init(req))
}

/// See [`Client::execute`]
Expand All @@ -134,6 +168,7 @@ impl From<Client> for ClientWithMiddleware {
ClientWithMiddleware {
inner: client,
middleware_stack: Box::new([]),
initialiser_stack: Box::new([]),
}
}
}
Expand All @@ -152,6 +187,7 @@ impl fmt::Debug for ClientWithMiddleware {
pub struct RequestBuilder {
inner: reqwest::RequestBuilder,
client: ClientWithMiddleware,
extensions: Extensions,
}

impl RequestBuilder {
Expand All @@ -164,14 +200,14 @@ impl RequestBuilder {
{
RequestBuilder {
inner: self.inner.header(key, value),
client: self.client,
..self
}
}

pub fn headers(self, headers: HeaderMap) -> Self {
RequestBuilder {
inner: self.inner.headers(headers),
client: self.client,
..self
}
}

Expand All @@ -182,7 +218,7 @@ impl RequestBuilder {
{
RequestBuilder {
inner: self.inner.basic_auth(username, password),
client: self.client,
..self
}
}

Expand All @@ -192,72 +228,94 @@ impl RequestBuilder {
{
RequestBuilder {
inner: self.inner.bearer_auth(token),
client: self.client,
..self
}
}

pub fn body<T: Into<Body>>(self, body: T) -> Self {
RequestBuilder {
inner: self.inner.body(body),
client: self.client,
..self
}
}

pub fn timeout(self, timeout: Duration) -> Self {
RequestBuilder {
inner: self.inner.timeout(timeout),
client: self.client,
..self
}
}

pub fn multipart(self, multipart: Form) -> Self {
RequestBuilder {
inner: self.inner.multipart(multipart),
client: self.client,
..self
}
}

pub fn query<T: Serialize + ?Sized>(self, query: &T) -> Self {
RequestBuilder {
inner: self.inner.query(query),
client: self.client,
..self
}
}

pub fn form<T: Serialize + ?Sized>(self, form: &T) -> Self {
RequestBuilder {
inner: self.inner.form(form),
client: self.client,
..self
}
}

pub fn json<T: Serialize + ?Sized>(self, json: &T) -> Self {
RequestBuilder {
inner: self.inner.json(json),
client: self.client,
..self
}
}

pub fn build(self) -> reqwest::Result<Request> {
self.inner.build()
}

/// Inserts the extension into this request builder
pub fn with_extension<T: Send + Sync + 'static>(mut self, extension: T) -> Self {
self.extensions.insert(extension);
self
}

/// Returns a mutable reference to the internal set of extensions for this request
pub fn extensions(&mut self) -> &mut Extensions {
&mut self.extensions
}

pub async fn send(self) -> Result<Response> {
let req = self.inner.build()?;
self.client.execute(req).await
let Self {
inner,
client,
mut extensions,
} = self;
let req = inner.build()?;
client.execute_with_extensions(req, &mut extensions).await
}

/// Sends a request with initial [`Extensions`].
#[deprecated = "use the with_extension method and send directly"]
pub async fn send_with_extensions(self, ext: &mut Extensions) -> Result<Response> {
let req = self.inner.build()?;
self.client.execute_with_extensions(req, ext).await
let Self { inner, client, .. } = self;
let req = inner.build()?;
client.execute_with_extensions(req, ext).await
}

// TODO(conradludgate): fix this method to take `&self`. It's currently useless as it is.
// I'm tempted to make this breaking change without a major bump, but I'll wait for now
#[deprecated = "This method was badly replicated from the base RequestBuilder. If you somehow made use of this method, it will break next major version"]
pub fn try_clone(self) -> Option<Self> {
let client = self.client;
self.inner
.try_clone()
.map(|inner| RequestBuilder { inner, client })
self.inner.try_clone().map(|inner| RequestBuilder {
inner,
client: self.client,
extensions: self.extensions,
})
}
}

Expand Down
2 changes: 2 additions & 0 deletions reqwest-middleware/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,9 @@ pub struct ReadmeDoctests;
mod client;
mod error;
mod middleware;
mod req_init;

pub use client::{ClientBuilder, ClientWithMiddleware, RequestBuilder};
pub use error::{Error, Result};
pub use middleware::{Middleware, Next};
pub use req_init::{Extension, RequestInitialiser};
84 changes: 84 additions & 0 deletions reqwest-middleware/src/req_init.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
use crate::RequestBuilder;

/// When attached to a [`ClientWithMiddleware`] (generally using [`with_init`]), it is run
/// whenever the client starts building a request, in the order it was attached.
///
/// # Example
///
/// ```
/// use reqwest_middleware::{RequestInitialiser, RequestBuilder};
///
/// struct AuthInit;
///
/// impl RequestInitialiser for AuthInit {
/// fn init(&self, req: RequestBuilder) -> RequestBuilder {
/// req.bearer_auth("my_auth_token")
/// }
/// }
Comment on lines +13 to +17
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can already be done with existing middleware via Request::headers_mut, maybe we should use a more motivating example that's harder to do with the Request struct?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can probably elaborate more - this is a client wide setting. Imagine you have a client that has many endpoints but all use the same auth settings, then those can be define in a RequestInitialiser on the top level connection

/// ```
///
/// [`ClientWithMiddleware`]: crate::ClientWithMiddleware
/// [`with_init`]: crate::ClientBuilder::with_init
pub trait RequestInitialiser: 'static + Send + Sync {
fn init(&self, req: RequestBuilder) -> RequestBuilder;
}

impl<F> RequestInitialiser for F
where
F: Send + Sync + 'static + Fn(RequestBuilder) -> RequestBuilder,
{
fn init(&self, req: RequestBuilder) -> RequestBuilder {
(self)(req)
}
}

/// A middleware that inserts the value into the [`Extensions`](task_local_extensions::Extensions) during the call.
///
/// This is a good way to inject extensions to middleware deeper in the stack
///
/// ```
/// use reqwest::{Client, Request, Response};
/// use reqwest_middleware::{ClientBuilder, Middleware, Next, Result, Extension};
/// use task_local_extensions::Extensions;
///
/// #[derive(Clone)]
/// struct LogName(&'static str);
/// struct LoggingMiddleware;
///
/// #[async_trait::async_trait]
/// impl Middleware for LoggingMiddleware {
/// async fn handle(
/// &self,
/// req: Request,
/// extensions: &mut Extensions,
/// next: Next<'_>,
/// ) -> Result<Response> {
/// // get the log name or default to "unknown"
/// let name = extensions
/// .get()
/// .map(|&LogName(name)| name)
/// .unwrap_or("unknown");
/// println!("[{name}] Request started {req:?}");
/// let res = next.run(req, extensions).await;
/// println!("[{name}] Result: {res:?}");
/// res
/// }
/// }
///
/// async fn run() {
/// let reqwest_client = Client::builder().build().unwrap();
/// let client = ClientBuilder::new(reqwest_client)
/// .with_init(Extension(LogName("my-client")))
/// .with(LoggingMiddleware)
/// .build();
Comment on lines +49 to +73
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this example couldn't the LogName have been baked into LoggingMiddleware. Or if the latter was a shared thing, a wrapper middleware used. I'm not seeing the motivating case for the new abstractions from this example very clearly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I thought I added the extra example. The case is where you might have a client wide extension, and then you can have a per-request configuration too.

let client = ClientBuilder::new(reqwest_client)
    .with_init(Extension(LogName("my-client")))
    .with(LoggingMiddleware)
    .build();
    
// uses "my-client" as the log name
client.get("https://truelayer.com").send().await

// uses "create-payment" as the log name
client.post("https://api.truelayer.com/payment")
    .with_extension(LogName("create-payment"))
    .send()
    .await

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this is something I want to supplement #54 because I'm not keen on breaking the current SpanBackend trait to add &self just yet, but it does have access to the extensions.

The desired configuration by the author was to have client wide configuration of the span name, but I think a request specific span name might be more desirable in some situations. So this is the best of both worlds for configuration

/// let resp = client.get("https://truelayer.com").send().await.unwrap();
/// println!("TrueLayer page HTML: {}", resp.text().await.unwrap());
/// }
/// ```
pub struct Extension<T>(pub T);

impl<T: Send + Sync + Clone + 'static> RequestInitialiser for Extension<T> {
fn init(&self, req: RequestBuilder) -> RequestBuilder {
req.with_extension(self.0.clone())
}
}