Repository: seanmonstar/reqwest (language: rust)

Issue:
Feature Request: `ResponseBuilder`
Related to this comment: https://github.com/seanmonstar/reqwest/issues/154#issuecomment-415962705

It would be nice to be able to manually build `Response` objects so we can mock out the HTTP interactions in tests


Discussion / hints:
Hm, perhaps adding a `From<http::Response> for reqwest::Response`, or something like that?
Yea, that would be perfect, actually
I've been trying to write this, and have run up against a couple of issues. First, I planned on actually writing two `From<http::Response>` implementations, one for `async_impl::Response` and one for `Response`. The impl for `async_impl::Response` is fairly straightforward, except for the fact that `async_impl::Response` expects a `Box<Url>` with the final URL. `http::Response` doesn't have this data (or at least, doesn't expose it through it's API), so right now I'm using the `.extensions()` mechanism to retrieve a `Url` from the `http::Response`. However, that returns an `Option` so I'm not sure what to do if there is no `Url`. Some sentinel value perhaps?

The other issue is that `Response` carries a reference to the `InnerClientHandle` through the `KeepCoreThreadAlive` type, and I have no idea how to get that from just a `From<http::Response>` impl.

Do you have any thoughts on this @seanmonstar?
Yea, I noticed those too when suggesting it...

- Looking in `extensions` for a `Url` is also what I was thinking... I'd suggest it should be a unique wrapper newtype, since `Url` by itself could have different meanings if extensions were filled by other libraries. If it doesn't exist, a sentinel value could make sense...
- The `KeepCoreThreadAlive` type could be changed to hold an `Option<Arc<InnerClientHandle>>`, and then an empty one can be created.

Fix patch:
diff --git a/src/async_impl/mod.rs b/src/async_impl/mod.rs
index 5f71a9713..cbeed0fb2 100644
--- a/src/async_impl/mod.rs
+++ b/src/async_impl/mod.rs
@@ -2,7 +2,7 @@ pub use self::body::{Body, Chunk};
 pub use self::decoder::{Decoder, ReadableChunks};
 pub use self::client::{Client, ClientBuilder};
 pub use self::request::{Request, RequestBuilder};
-pub use self::response::Response;
+pub use self::response::{Response, ResponseBuilderExt};
 
 pub mod body;
 pub mod client;
diff --git a/src/async_impl/response.rs b/src/async_impl/response.rs
index 942b26376..7dd11fbd4 100644
--- a/src/async_impl/response.rs
+++ b/src/async_impl/response.rs
@@ -8,6 +8,7 @@ use hyper::{HeaderMap, StatusCode, Version};
 use serde::de::DeserializeOwned;
 use serde_json;
 use url::Url;
+use http;
 
 use super::{decoder, body, Decoder};
 
@@ -144,6 +145,25 @@ impl fmt::Debug for Response {
     }
 }
 
+impl<T: Into<body::Body>> From<http::Response<T>> for Response {
+    fn from(r: http::Response<T>) -> Response {
+        let (mut parts, body) = r.into_parts();
+        let body = body.into();
+        let body = decoder::detect(&mut parts.headers, body, false);
+        let url = parts.extensions
+            .remove::<ResponseUrl>()
+            .unwrap_or_else(|| ResponseUrl(Url::parse("http://no.url.provided.local").unwrap()));
+        let url = url.0;
+        Response {
+            status: parts.status,
+            headers: parts.headers,
+            url: Box::new(url),
+            body: body,
+            version: parts.version,
+        }
+    }
+}
+
 pub struct Json<T> {
     concat: Concat2<Decoder>,
     _marker: PhantomData<T>,
@@ -166,3 +186,72 @@ impl<T> fmt::Debug for Json<T> {
     }
 }
 
+#[derive(Debug, Clone, PartialEq)]
+struct ResponseUrl(Url);
+
+/// Extension trait for http::response::Builder objects
+///
+/// Allows the user to add a `Url` to the http::Response
+pub trait ResponseBuilderExt {
+    /// A builder method for the `http::response::Builder` type that allows the user to add a `Url`
+    /// to the `http::Response`
+    ///
+    /// # Example
+    ///
+    /// ```
+    /// # extern crate url;
+    /// # extern crate http;
+    /// # extern crate reqwest;
+    /// # use std::error::Error;
+    /// use url::Url;
+    /// use http::response::Builder;
+    /// use reqwest::async::ResponseBuilderExt;
+    /// # fn main() -> Result<(), Box<Error>> {
+    /// let response = Builder::new()
+    ///     .status(200)
+    ///     .url(Url::parse("http://example.com")?)
+    ///     .body(())?;
+    ///
+    /// #   Ok(())
+    /// # }
+    fn url(&mut self, url: Url) -> &mut Self;
+}
+
+impl ResponseBuilderExt for http::response::Builder {
+    fn url(&mut self, url: Url) -> &mut Self {
+        self.extension(ResponseUrl(url))
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use url::Url;
+    use http::response::Builder;
+    use super::{Response, ResponseUrl, ResponseBuilderExt};
+
+    #[test]
+    fn test_response_builder_ext() {
+        let url = Url::parse("http://example.com").unwrap();
+        let response = Builder::new()
+            .status(200)
+            .url(url.clone())
+            .body(())
+            .unwrap();
+
+        assert_eq!(response.extensions().get::<ResponseUrl>(), Some(&ResponseUrl(url)));
+    }
+
+    #[test]
+    fn test_from_http_response() {
+        let url = Url::parse("http://example.com").unwrap();
+        let response = Builder::new()
+            .status(200)
+            .url(url.clone())
+            .body("foo")
+            .unwrap();
+        let response = Response::from(response);
+
+        assert_eq!(response.status, 200);
+        assert_eq!(response.url, Box::new(url));
+    }
+}
diff --git a/src/client.rs b/src/client.rs
index 19a4358bf..f59f77710 100644
--- a/src/client.rs
+++ b/src/client.rs
@@ -500,7 +500,7 @@ impl ClientHandle {
             }
         };
         res.map(|res| {
-            response::new(res, self.timeout.0, KeepCoreThreadAlive(self.inner.clone()))
+            response::new(res, self.timeout.0, KeepCoreThreadAlive(Some(self.inner.clone())))
         })
     }
 }
@@ -517,4 +517,10 @@ impl Default for Timeout {
 
 // pub(crate)
 
-pub struct KeepCoreThreadAlive(Arc<InnerClientHandle>);
+pub struct KeepCoreThreadAlive(Option<Arc<InnerClientHandle>>);
+
+impl KeepCoreThreadAlive {
+    pub(crate) fn empty() -> KeepCoreThreadAlive {
+        KeepCoreThreadAlive(None)
+    }
+}
diff --git a/src/lib.rs b/src/lib.rs
index 5f5042e10..fe159cf22 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -203,6 +203,7 @@ pub mod async {
         Request,
         RequestBuilder,
         Response,
+        ResponseBuilderExt,
     };
 }
 
diff --git a/src/response.rs b/src/response.rs
index 9373945a7..bfe5c8d3b 100644
--- a/src/response.rs
+++ b/src/response.rs
@@ -6,6 +6,7 @@ use std::borrow::Cow;
 
 use encoding_rs::{Encoding, UTF_8};
 use futures::{Async, Poll, Stream};
+use http;
 use mime::Mime;
 use serde::de::DeserializeOwned;
 use serde_json;
@@ -336,3 +337,9 @@ pub fn new(mut res: async_impl::Response, timeout: Option<Duration>, thread: Kee
     }
 }
 
+impl<T: Into<async_impl::body::Body>> From<http::Response<T>> for Response {
+    fn from(r: http::Response<T>) -> Response {
+        let response = async_impl::Response::from(r);
+        new(response, None, KeepCoreThreadAlive::empty())
+    }
+}


Test patch: