Agent tweaks for 2.0

Replace `this` idiom with `mut self`.

Move idle connections constants from pool.rs to agent.rs.

Remove Agent.set and some convenience request methods
(leaving get, post, and put).

Move max_idle_connections setting from AgentConfig to AgentBuilder
(since the builder passes these to ConnectionPool and the Agent
doesn't subsequently need them).

Eliminate duplicate copy of proxy in AgentState; use the one in
AgentConfig.
This commit is contained in:
Jacob Hoffman-Andrews
2020-10-28 23:55:30 -07:00
committed by Martin Algesten
parent e3bf7c73f3
commit aae05c5614
3 changed files with 29 additions and 90 deletions

View File

@@ -14,6 +14,8 @@ use {crate::cookies::CookieTin, cookie::Cookie, cookie_store::CookieStore, url::
pub struct AgentBuilder { pub struct AgentBuilder {
headers: Vec<Header>, headers: Vec<Header>,
config: AgentConfig, config: AgentConfig,
max_idle_connections: usize,
max_idle_connections_per_host: usize,
/// Cookies saved between requests. /// Cookies saved between requests.
/// Invariant: All cookies must have a nonempty domain and path. /// Invariant: All cookies must have a nonempty domain and path.
#[cfg(feature = "cookies")] #[cfg(feature = "cookies")]
@@ -24,8 +26,6 @@ pub struct AgentBuilder {
/// Config as built by AgentBuilder and then static for the lifetime of the Agent. /// Config as built by AgentBuilder and then static for the lifetime of the Agent.
#[derive(Debug, Clone)] #[derive(Debug, Clone)]
pub(crate) struct AgentConfig { pub(crate) struct AgentConfig {
pub max_idle_connections: usize,
pub max_idle_connections_per_host: usize,
pub proxy: Option<Proxy>, pub proxy: Option<Proxy>,
pub timeout_connect: Option<Duration>, pub timeout_connect: Option<Duration>,
pub timeout_read: Option<Duration>, pub timeout_read: Option<Duration>,
@@ -45,8 +45,6 @@ pub(crate) struct AgentConfig {
/// ``` /// ```
/// let mut agent = ureq::agent(); /// let mut agent = ureq::agent();
/// ///
/// agent.set("x-my-secret-header", "very secret");
///
/// let auth = agent /// let auth = agent
/// .post("/login") /// .post("/login")
/// .call(); // blocks. /// .call(); // blocks.
@@ -84,7 +82,6 @@ pub struct Agent {
pub(crate) struct AgentState { pub(crate) struct AgentState {
/// Reused connections between requests. /// Reused connections between requests.
pub(crate) pool: ConnectionPool, pub(crate) pool: ConnectionPool,
pub(crate) proxy: Option<Proxy>,
/// Cookies saved between requests. /// Cookies saved between requests.
/// Invariant: All cookies must have a nonempty domain and path. /// Invariant: All cookies must have a nonempty domain and path.
#[cfg(feature = "cookies")] #[cfg(feature = "cookies")]
@@ -100,27 +97,6 @@ impl Agent {
AgentBuilder::new().build() AgentBuilder::new().build()
} }
/// Set a extra header field that will be present in all following requests using the agent.
///
/// This is useful for cases like auth, where we do a number of requests before getting
/// some credential that later must be presented in a header.
///
/// Notice that fixed headers can also be set in the `AgentBuilder`.
///
/// ```
/// let mut agent = ureq::agent();
///
/// agent.set("X-API-Key", "foobar");
/// agent.set("Accept", "text/plain");
///
/// let r = agent
/// .get("/my-page")
/// .call();
/// ```
pub fn set(&mut self, header: &str, value: &str) {
header::add_header(&mut self.headers, Header::new(header, value));
}
/// Request by providing the HTTP verb such as `GET`, `POST`... /// Request by providing the HTTP verb such as `GET`, `POST`...
/// ///
/// ``` /// ```
@@ -157,11 +133,6 @@ impl Agent {
self.request("GET", path) self.request("GET", path)
} }
/// Make a HEAD request from this agent.
pub fn head(&self, path: &str) -> Request {
self.request("HEAD", path)
}
/// Make a POST request from this agent. /// Make a POST request from this agent.
pub fn post(&self, path: &str) -> Request { pub fn post(&self, path: &str) -> Request {
self.request("POST", path) self.request("POST", path)
@@ -171,35 +142,16 @@ impl Agent {
pub fn put(&self, path: &str) -> Request { pub fn put(&self, path: &str) -> Request {
self.request("PUT", path) self.request("PUT", path)
} }
/// Make a DELETE request from this agent.
pub fn delete(&self, path: &str) -> Request {
self.request("DELETE", path)
} }
/// Make a TRACE request from this agent. const DEFAULT_MAX_IDLE_CONNECTIONS: usize = 100;
pub fn trace(&self, path: &str) -> Request { const DEFAULT_MAX_IDLE_CONNECTIONS_PER_HOST: usize = 1;
self.request("TRACE", path)
}
/// Make a OPTIONS request from this agent.
pub fn options(&self, path: &str) -> Request {
self.request("OPTIONS", path)
}
/// Make a PATCH request from this agent.
pub fn patch(&self, path: &str) -> Request {
self.request("PATCH", path)
}
}
impl AgentBuilder { impl AgentBuilder {
pub fn new() -> Self { pub fn new() -> Self {
AgentBuilder { AgentBuilder {
headers: vec![], headers: vec![],
config: AgentConfig { config: AgentConfig {
max_idle_connections: crate::pool::DEFAULT_MAX_IDLE_CONNECTIONS,
max_idle_connections_per_host: crate::pool::DEFAULT_MAX_IDLE_CONNECTIONS_PER_HOST,
proxy: None, proxy: None,
timeout_connect: Some(Duration::from_secs(30)), timeout_connect: Some(Duration::from_secs(30)),
timeout_read: None, timeout_read: None,
@@ -209,6 +161,8 @@ impl AgentBuilder {
#[cfg(feature = "tls")] #[cfg(feature = "tls")]
tls_config: None, tls_config: None,
}, },
max_idle_connections: DEFAULT_MAX_IDLE_CONNECTIONS,
max_idle_connections_per_host: DEFAULT_MAX_IDLE_CONNECTIONS_PER_HOST,
resolver: StdResolver.into(), resolver: StdResolver.into(),
#[cfg(feature = "cookies")] #[cfg(feature = "cookies")]
cookie_store: None, cookie_store: None,
@@ -221,22 +175,20 @@ impl AgentBuilder {
// not implement clone, so we have to give ownership to the newly // not implement clone, so we have to give ownership to the newly
// built Agent. // built Agent.
pub fn build(self) -> Agent { pub fn build(self) -> Agent {
let config = Arc::new(self.config);
Agent { Agent {
headers: self.headers, headers: self.headers,
config: Arc::new(self.config),
state: Arc::new(AgentState { state: Arc::new(AgentState {
pool: ConnectionPool::new_with_limits( pool: ConnectionPool::new_with_limits(
config.max_idle_connections, self.max_idle_connections,
config.max_idle_connections_per_host, self.max_idle_connections_per_host,
), ),
proxy: config.proxy.clone(),
#[cfg(feature = "cookies")] #[cfg(feature = "cookies")]
cookie_tin: CookieTin::new( cookie_tin: CookieTin::new(
self.cookie_store.unwrap_or_else(|| CookieStore::default()), self.cookie_store.unwrap_or_else(|| CookieStore::default()),
), ),
resolver: self.resolver, resolver: self.resolver,
}), }),
config,
} }
} }
@@ -285,7 +237,7 @@ impl AgentBuilder {
/// let agent = ureq::AgentBuilder::new().max_idle_connections(200).build(); /// let agent = ureq::AgentBuilder::new().max_idle_connections(200).build();
/// ``` /// ```
pub fn max_idle_connections(mut self, max: usize) -> Self { pub fn max_idle_connections(mut self, max: usize) -> Self {
self.config.max_idle_connections = max; self.max_idle_connections = max;
self self
} }
@@ -297,7 +249,7 @@ impl AgentBuilder {
/// let agent = ureq::AgentBuilder::new().max_idle_connections_per_host(200).build(); /// let agent = ureq::AgentBuilder::new().max_idle_connections_per_host(200).build();
/// ``` /// ```
pub fn max_idle_connections_per_host(mut self, max: usize) -> Self { pub fn max_idle_connections_per_host(mut self, max: usize) -> Self {
self.config.max_idle_connections_per_host = max; self.max_idle_connections_per_host = max;
self self
} }

View File

@@ -9,9 +9,6 @@ use crate::Proxy;
use url::Url; use url::Url;
pub const DEFAULT_MAX_IDLE_CONNECTIONS: usize = 100;
pub const DEFAULT_MAX_IDLE_CONNECTIONS_PER_HOST: usize = 1;
/// Holder of recycled connections. /// Holder of recycled connections.
/// ///
/// For each PoolKey (approximately hostname and port), there may be /// For each PoolKey (approximately hostname and port), there may be
@@ -75,14 +72,6 @@ fn remove_last_match(list: &mut VecDeque<PoolKey>, key: &PoolKey) -> Option<Pool
} }
impl ConnectionPool { impl ConnectionPool {
#[cfg(test)]
pub(crate) fn new() -> Self {
Self::new_with_limits(
DEFAULT_MAX_IDLE_CONNECTIONS,
DEFAULT_MAX_IDLE_CONNECTIONS_PER_HOST,
)
}
pub(crate) fn new_with_limits( pub(crate) fn new_with_limits(
max_idle_connections: usize, max_idle_connections: usize,
max_idle_connections_per_host: usize, max_idle_connections_per_host: usize,
@@ -231,8 +220,8 @@ fn pool_connections_limit() {
// Test inserting connections with different keys into the pool, // Test inserting connections with different keys into the pool,
// filling and draining it. The pool should evict earlier connections // filling and draining it. The pool should evict earlier connections
// when the connection limit is reached. // when the connection limit is reached.
let pool = ConnectionPool::new(); let pool = ConnectionPool::new_with_limits(10, 1);
let hostnames = (0..DEFAULT_MAX_IDLE_CONNECTIONS * 2).map(|i| format!("{}.example", i)); let hostnames = (0..pool.max_idle_connections * 2).map(|i| format!("{}.example", i));
let poolkeys = hostnames.map(|hostname| PoolKey { let poolkeys = hostnames.map(|hostname| PoolKey {
scheme: "https".to_string(), scheme: "https".to_string(),
hostname, hostname,
@@ -242,9 +231,9 @@ fn pool_connections_limit() {
for key in poolkeys.clone() { for key in poolkeys.clone() {
pool.add(key, Stream::Cursor(std::io::Cursor::new(vec![]))); pool.add(key, Stream::Cursor(std::io::Cursor::new(vec![])));
} }
assert_eq!(pool.len(), DEFAULT_MAX_IDLE_CONNECTIONS); assert_eq!(pool.len(), pool.max_idle_connections);
for key in poolkeys.skip(DEFAULT_MAX_IDLE_CONNECTIONS) { for key in poolkeys.skip(pool.max_idle_connections) {
let result = pool.remove(&key); let result = pool.remove(&key);
assert!(result.is_some(), "expected key was not in pool"); assert!(result.is_some(), "expected key was not in pool");
} }
@@ -256,7 +245,7 @@ fn pool_per_host_connections_limit() {
// Test inserting connections with the same key into the pool, // Test inserting connections with the same key into the pool,
// filling and draining it. The pool should evict earlier connections // filling and draining it. The pool should evict earlier connections
// when the per-host connection limit is reached. // when the per-host connection limit is reached.
let pool = ConnectionPool::new(); let pool = ConnectionPool::new_with_limits(10, 2);
let poolkey = PoolKey { let poolkey = PoolKey {
scheme: "https".to_string(), scheme: "https".to_string(),
hostname: "example.com".to_string(), hostname: "example.com".to_string(),
@@ -270,9 +259,9 @@ fn pool_per_host_connections_limit() {
Stream::Cursor(std::io::Cursor::new(vec![])), Stream::Cursor(std::io::Cursor::new(vec![])),
); );
} }
assert_eq!(pool.len(), DEFAULT_MAX_IDLE_CONNECTIONS_PER_HOST); assert_eq!(pool.len(), pool.max_idle_connections_per_host);
for _ in 0..DEFAULT_MAX_IDLE_CONNECTIONS_PER_HOST { for _ in 0..pool.max_idle_connections_per_host {
let result = pool.remove(&poolkey); let result = pool.remove(&poolkey);
assert!(result.is_some(), "expected key was not in pool"); assert!(result.is_some(), "expected key was not in pool");
} }
@@ -283,7 +272,7 @@ fn pool_per_host_connections_limit() {
fn pool_checks_proxy() { fn pool_checks_proxy() {
// Test inserting different poolkeys with same address but different proxies. // Test inserting different poolkeys with same address but different proxies.
// Each insertion should result in an additional entry in the pool. // Each insertion should result in an additional entry in the pool.
let pool = ConnectionPool::new(); let pool = ConnectionPool::new_with_limits(10, 1);
let url = Url::parse("zzz:///example.com").unwrap(); let url = Url::parse("zzz:///example.com").unwrap();
pool.add( pool.add(

View File

@@ -118,12 +118,11 @@ impl Request {
/// } /// }
/// ``` /// ```
#[cfg(feature = "json")] #[cfg(feature = "json")]
pub fn send_json(self, data: SerdeValue) -> Result<Response> { pub fn send_json(mut self, data: SerdeValue) -> Result<Response> {
let mut this = self; if self.header("Content-Type").is_none() {
if this.header("Content-Type").is_none() { self = self.set("Content-Type", "application/json");
this = this.set("Content-Type", "application/json");
} }
this.do_call(Payload::JSON(data)) self.do_call(Payload::JSON(data))
} }
/// Send data as bytes. /// Send data as bytes.
@@ -182,15 +181,14 @@ impl Request {
/// println!("{:?}", r); /// println!("{:?}", r);
/// } /// }
/// ``` /// ```
pub fn send_form(self, data: &[(&str, &str)]) -> Result<Response> { pub fn send_form(mut self, data: &[(&str, &str)]) -> Result<Response> {
let mut this = self; if self.header("Content-Type").is_none() {
if this.header("Content-Type").is_none() { self = self.set("Content-Type", "application/x-www-form-urlencoded");
this = this.set("Content-Type", "application/x-www-form-urlencoded");
} }
let encoded = form_urlencoded::Serializer::new(String::new()) let encoded = form_urlencoded::Serializer::new(String::new())
.extend_pairs(data) .extend_pairs(data)
.finish(); .finish();
this.do_call(Payload::Bytes(&encoded.into_bytes())) self.do_call(Payload::Bytes(&encoded.into_bytes()))
} }
/// Send data from a reader. /// Send data from a reader.
@@ -423,7 +421,7 @@ impl Request {
} }
pub(crate) fn proxy(&self) -> Option<Proxy> { pub(crate) fn proxy(&self) -> Option<Proxy> {
if let Some(proxy) = &self.agent.state.proxy { if let Some(proxy) = &self.agent.config.proxy {
Some(proxy.clone()) Some(proxy.clone())
} else { } else {
None None