Skip to content

Commit

Permalink
Implement default headers for #180 (#181)
Browse files Browse the repository at this point in the history 
* Might fix #184

Signed-off-by: Ankur Srivastava <[email protected]>

* handle review

Signed-off-by: Ankur Srivastava <[email protected]>

* more test cases

Signed-off-by: Ankur Srivastava <[email protected]>

* trying to match with Any in tests

Signed-off-by: Ankur Srivastava <[email protected]>

* use keys to iterate over HeaderMap

Signed-off-by: Ankur Srivastava <[email protected]>

* removed DefaultHeaderMap implementation, not using .set_opt anymore

Signed-off-by: Ankur Srivastava <[email protected]>

* format

Signed-off-by: Ankur Srivastava <[email protected]>

* review comments:
- Keep the error as enum http::Error
- Commented out the failing tests until it gets fixed upstream in mockito

Signed-off-by: Ankur Srivastava <[email protected]>
  • Loading branch information
@ansrivas
ansrivas authored May 3, 2020
1 parent 3fdccc6 commit 1471674
Show file tree
Hide file tree
Showing 3 changed files with 236 additions and 1 deletion.
106 changes: 105 additions & 1 deletion src/client.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use crate::{
};
use futures_io::AsyncRead;
use futures_util::{future::BoxFuture, pin_mut};
use http::header::{HeaderName, HeaderValue};
use http::{Request, Response};
use lazy_static::lazy_static;
use std::{
Expand Down Expand Up @@ -58,6 +59,8 @@ pub struct HttpClientBuilder {
agent_builder: AgentBuilder,
defaults: http::Extensions,
middleware: Vec<Box<dyn Middleware>>,
default_headers: http::HeaderMap<HeaderValue>,
error: Option<Error>,
}

impl Default for HttpClientBuilder {
Expand All @@ -84,6 +87,8 @@ impl HttpClientBuilder {
agent_builder: AgentBuilder::default(),
defaults,
middleware: Vec::new(),
default_headers: http::HeaderMap::new(),
error: None,
}
}

Expand Down Expand Up @@ -240,14 +245,72 @@ impl HttpClientBuilder {
self.configure(map)
}

/// Set a default header to be passed with every request
///
/// NOTE: In case there is an error in parsing the HeaderName or HeaderValue
/// the tuple is silently discarded.
///
/// # Examples
///
/// ```
/// # use isahc::prelude::*;
/// #
/// let client = HttpClient::builder()
/// .default_header("some-header", "some-value")
/// .build()?;
/// # Ok::<(), Box<dyn std::error::Error>>(())
/// ```
pub fn default_header<K, V>(mut self, key: K, value: V) -> Self
where
HeaderName: TryFrom<K>,
HeaderValue: TryFrom<V>,
<HeaderName as TryFrom<K>>::Error: Into<Error>,
<HeaderValue as TryFrom<V>>::Error: Into<Error>,
{
match HeaderName::try_from(key) {
Ok(key) => match HeaderValue::try_from(value) {
Ok(value) => {
self.default_headers.append(key, value);
}
Err(e) => {
self.error = Some(e.into());
}
},
Err(e) => {
self.error = Some(e.into());
}
}
self
}

/// Get the underlying HeaderMap from the current client-builder.
///
/// # Examples
///
/// ```
/// # use isahc::prelude::*;
/// #
/// let mut builder = HttpClient::builder()
/// .default_header("some-header", "some-value");
/// let header_opt = builder.default_headers_mut();
/// # Ok::<(), Box<dyn std::error::Error>>(())
/// ```
pub fn default_headers_mut(&mut self) -> &mut http::HeaderMap<HeaderValue> {
&mut self.default_headers
}

/// Build an [`HttpClient`] using the configured options.
///
/// If the client fails to initialize, an error will be returned.
pub fn build(self) -> Result<HttpClient, Error> {
if let Some(err) = self.error {
return Err(err);
}
Ok(HttpClient {
agent: Arc::new(self.agent_builder.spawn()?),
defaults: self.defaults,
middleware: self.middleware,
default_headers: self.default_headers,
})
}
}
Expand Down Expand Up @@ -343,6 +406,8 @@ pub struct HttpClient {
defaults: http::Extensions,
/// Any middleware implementations that requests should pass through.
middleware: Vec<Box<dyn Middleware>>,

default_headers: http::HeaderMap<HeaderValue>,
}

impl HttpClient {
Expand Down Expand Up @@ -827,7 +892,17 @@ impl HttpClient {
}
}

// Set custom request headers.
// We are checking here if header already contains the key, simply ignore it.
// In case the key wasn't present in parts.headers ensure that
// we have all the headers from default headers.
for name in self.default_headers.keys() {
if !parts.headers.contains_key(name) {
for v in self.default_headers.get_all(name).iter() {
parts.headers.append(name, v.clone());
}
}
}

parts.headers.set_opt(&mut easy)?;

Ok((easy, future))
Expand Down Expand Up @@ -889,4 +964,33 @@ mod tests {

static_assertions::assert_impl_all!(HttpClient: Send, Sync);
static_assertions::assert_impl_all!(HttpClientBuilder: Send);

#[test]
fn test_default_header() {
let client = HttpClientBuilder::new()
.default_header("some-key", "some-value")
.build();
match client {
Ok(_) => assert!(true),
Err(_) => assert!(false),
}
}

#[test]
fn test_default_headers_mut() {
let mut builder = HttpClientBuilder::new().default_header("some-key", "some-value");
let headers_map = builder.default_headers_mut();
assert!(headers_map.len() == 1);

let mut builder = HttpClientBuilder::new()
.default_header("some-key", "some-value1")
.default_header("some-key", "some-value2");
let headers_map = builder.default_headers_mut();

assert!(headers_map.len() == 2);

let mut builder = HttpClientBuilder::new();
let header_map = builder.default_headers_mut();
assert!(header_map.is_empty())
}
}
14 changes: 14 additions & 0 deletions src/error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,20 @@ impl From<http::Error> for Error {
}
}

#[doc(hidden)]
impl From<http::header::InvalidHeaderName> for Error {
fn from(error: http::header::InvalidHeaderName) -> Error {
Error::InvalidHttpFormat(error.into())
}
}

#[doc(hidden)]
impl From<http::header::InvalidHeaderValue> for Error {
fn from(error: http::header::InvalidHeaderValue) -> Error {
Error::InvalidHttpFormat(error.into())
}
}

#[doc(hidden)]
impl From<io::Error> for Error {
fn from(error: io::Error) -> Error {
Expand Down
117 changes: 117 additions & 0 deletions tests/headers.rs
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use isahc::prelude::*;
use mockito::{mock, server_url, Matcher};

speculate::speculate! {
Expand Down Expand Up @@ -25,4 +26,120 @@ speculate::speculate! {

m.assert();
}

test "header can be inserted in HttpClient::builder" {

let host_header = server_url().replace("http://", "");
let m = mock("GET", "/")
.match_header("host", host_header.as_ref())
.match_header("accept", "*/*")
.match_header("accept-encoding", "deflate, gzip")
// .match_header("user-agent", Matcher::Regex(r"^curl/\S+ isahc/\S+$".into()))
.match_header("user-agent", Matcher::Any)
.match_header("X-header", "some-value1")
.create();

let client = HttpClient::builder()
.default_header("X-header", "some-value1")
.build()
.unwrap();

let request = Request::builder()
.method("GET")
.uri(server_url())
.body(())
.unwrap();

let _ = client.send(request).unwrap();
m.assert();
}

test "headers in Request::builder must override headers in HttpClient::builder" {

let host_header = server_url().replace("http://", "");
let m = mock("GET", "/")
.match_header("host", host_header.as_ref())
.match_header("accept", "*/*")
.match_header("accept-encoding", "deflate, gzip")
// .match_header("user-agent", Matcher::Regex(r"^curl/\S+ isahc/\S+$".into()))
.match_header("user-agent", Matcher::Any)
.match_header("X-header", "some-value2")
.create();

let client = HttpClient::builder()
.default_header("X-header", "some-value1")
.build()
.unwrap();

let request = Request::builder()
.method("GET")
.header("X-header", "some-value2")
.uri(server_url())
.body(())
.unwrap();

let _ = client.send(request).unwrap();
m.assert();
}

// test "multiple headers with same key can be inserted in HttpClient::builder" {

// let host_header = server_url().replace("http://", "");
// let m = mock("GET", "/")
// .match_header("host", host_header.as_ref())
// .match_header("accept", "*/*")
// .match_header("accept-encoding", "deflate, gzip")
// // .match_header("user-agent", Matcher::Regex(r"^curl/\S+ isahc/\S+$".into()))
// .match_header("user-agent", Matcher::Any)
// .match_header("X-header", "some-value1")
// .match_header("X-header", "some-value2")
// .create();

// let client = HttpClient::builder()
// .default_header("X-header", "some-value1")
// .default_header("X-header", "some-value2")
// .build()
// .unwrap();

// let request = Request::builder()
// .method("GET")
// .uri(server_url())
// .body(())
// .unwrap();

// let _ = client.send(request).unwrap();
// m.assert();
// }




test "headers in Request::builder must override multiple headers in HttpClient::builder" {

let host_header = server_url().replace("http://", "");
let m = mock("GET", "/")
.match_header("host", host_header.as_ref())
.match_header("accept", "*/*")
.match_header("accept-encoding", "deflate, gzip")
// .match_header("user-agent", Matcher::Regex(r"^curl/\S+ isahc/\S+$".into()))
.match_header("user-agent", Matcher::Any)
.match_header("X-header", "some-value3")
.create();

let client = HttpClient::builder()
.default_header("X-header", "some-value1")
.default_header("X-header", "some-value2")
.build()
.unwrap();

let request = Request::builder()
.method("GET")
.header("X-header", "some-value3")
.uri(server_url())
.body(())
.unwrap();

let _ = client.send(request).unwrap();
m.assert();
}
}

0 comments on commit 1471674

Please sign in to comment.