lib/ureq
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Turn Option<AgentState> into AgentState (#149)

Browse Source 
`Some(AgentState)` seem to be assumed pretty much everywhere. I could not
find any test or piece of code hinting at how `None` should be interpreted,
or even see how a state of `None` could even be constructed.

Co-authored-by: Ulrik <ulrikm@spotify.com>
This commit is contained in:
Ulrik Mikaelsson
2020-09-18 22:47:11 +02:00
committed by GitHub
parent 90d0b35ae5
commit f599828c6d
5 changed files with 43 additions and 67 deletions
Download Patch File Download Diff File Expand all files Collapse all files

37
src/agent.rs
View File

@@ -40,13 +40,13 @@ pub struct Agent {
/// Copied into each request of this agent. /// Copied into each request of this agent.
pub(crate) headers: Vec<Header>, pub(crate) headers: Vec<Header>,
/// Reused agent state for repeated requests from this agent. /// Reused agent state for repeated requests from this agent.
pub(crate) state: Arc<Mutex<Option<AgentState>>>, pub(crate) state: Arc<Mutex<AgentState>>,
} }
/// Container of the state /// Container of the state
/// ///
/// *Internal API*. /// *Internal API*.
#[derive(Debug)] #[derive(Debug, Default)]
pub(crate) struct AgentState { pub(crate) struct AgentState {
/// Reused connections between requests. /// Reused connections between requests.
pub(crate) pool: ConnectionPool, pub(crate) pool: ConnectionPool,
@@ -89,7 +89,7 @@ impl Agent {
pub fn build(&self) -> Self { pub fn build(&self) -> Self {
Agent { Agent {
headers: self.headers.clone(), headers: self.headers.clone(),
state: Arc::new(Mutex::new(Some(AgentState::new()))), state: Arc::new(Mutex::new(AgentState::new())),
} }
} }
@@ -175,10 +175,8 @@ impl Agent {
/// agent.set_max_pool_connections(200); /// agent.set_max_pool_connections(200);
/// ``` /// ```
pub fn set_max_pool_connections(&self, max_connections: usize) { pub fn set_max_pool_connections(&self, max_connections: usize) {
let mut optional_state = self.state.lock().unwrap(); let mut state = self.state.lock().unwrap();
if let Some(state) = optional_state.as_mut() { state.pool.set_max_idle_connections(max_connections);
state.pool.set_max_idle_connections(max_connections);
}
} }
/// Sets the maximum number of connections per host to keep in the /// Sets the maximum number of connections per host to keep in the
@@ -190,12 +188,10 @@ impl Agent {
/// agent.set_max_pool_connections_per_host(10); /// agent.set_max_pool_connections_per_host(10);
/// ``` /// ```
pub fn set_max_pool_connections_per_host(&self, max_connections: usize) { pub fn set_max_pool_connections_per_host(&self, max_connections: usize) {
let mut optional_state = self.state.lock().unwrap(); let mut state = self.state.lock().unwrap();
if let Some(state) = optional_state.as_mut() { state
state .pool
.pool .set_max_idle_connections_per_host(max_connections);
.set_max_idle_connections_per_host(max_connections);
}
} }
/// Gets a cookie in this agent by name. Cookies are available /// Gets a cookie in this agent by name. Cookies are available
@@ -212,10 +208,7 @@ impl Agent {
#[cfg(feature = "cookie")] #[cfg(feature = "cookie")]
pub fn cookie(&self, name: &str) -> Option<Cookie<'static>> { pub fn cookie(&self, name: &str) -> Option<Cookie<'static>> {
let state = self.state.lock().unwrap(); let state = self.state.lock().unwrap();
state state.jar.get(name).cloned()
.as_ref()
.and_then(|state| state.jar.get(name))
.cloned()
} }
/// Set a cookie in this agent. /// Set a cookie in this agent.
@@ -229,12 +222,7 @@ impl Agent {
#[cfg(feature = "cookie")] #[cfg(feature = "cookie")]
pub fn set_cookie(&self, cookie: Cookie<'static>) { pub fn set_cookie(&self, cookie: Cookie<'static>) {
let mut state = self.state.lock().unwrap(); let mut state = self.state.lock().unwrap();
match state.as_mut() { state.jar.add_original(cookie);
None => (),
Some(state) => {
state.jar.add_original(cookie);
}
}
} }
/// Make a GET request from this agent. /// Make a GET request from this agent.
@@ -321,8 +309,7 @@ mod tests {
reader.read_to_end(&mut buf).unwrap(); reader.read_to_end(&mut buf).unwrap();
fn poolsize(agent: &Agent) -> usize { fn poolsize(agent: &Agent) -> usize {
let mut lock = agent.state.lock().unwrap(); let mut state = agent.state.lock().unwrap();
let state = lock.as_mut().unwrap();
state.pool().len() state.pool().len()
} }
assert_eq!(poolsize(&agent), 1); assert_eq!(poolsize(&agent), 1);

14
src/pool.rs
View File

@@ -381,15 +381,13 @@ impl<R: Read + Sized + Into<Stream>> PoolReturnRead<R> {
let state = &mut unit.agent.lock().unwrap(); let state = &mut unit.agent.lock().unwrap();
// bring back stream here to either go into pool or dealloc // bring back stream here to either go into pool or dealloc
let stream = reader.into(); let stream = reader.into();
if let Some(agent) = state.as_mut() { if !stream.is_poolable() {
if !stream.is_poolable() { // just let it deallocate
// just let it deallocate return;
return;
}
// insert back into pool
let key = PoolKey::new(&unit.url, &unit.proxy);
agent.pool().add(key, stream);
} }
// insert back into pool
let key = PoolKey::new(&unit.url, &unit.proxy);
state.pool().add(key, stream);
} }
} }

2
src/request.rs
View File

@@ -29,7 +29,7 @@ use super::SerdeValue;
/// ``` /// ```
#[derive(Clone, Default)] #[derive(Clone, Default)]
pub struct Request { pub struct Request {
pub(crate) agent: Arc<Mutex<Option<AgentState>>>, pub(crate) agent: Arc<Mutex<AgentState>>,
// via agent // via agent
pub(crate) method: String, pub(crate) method: String,

3
src/test/agent_test.rs
View File

@@ -78,8 +78,7 @@ fn connection_reuse() {
resp.into_string().unwrap(); resp.into_string().unwrap();
{ {
let mut guard_state = agent.state.lock().unwrap(); let mut state = agent.state.lock().unwrap();
let mut state = guard_state.take().unwrap();
assert!(state.pool().len() > 0); assert!(state.pool().len() > 0);
} }

54
src/unit.rs
View File

@@ -27,9 +27,8 @@ use crate::pool::DEFAULT_HOST;
/// It's a "unit of work". Maybe a bad name for it? /// It's a "unit of work". Maybe a bad name for it?
/// ///
/// *Internal API* /// *Internal API*
#[derive(Debug)]
pub(crate) struct Unit { pub(crate) struct Unit {
pub agent: Arc<Mutex<Option<AgentState>>>, pub agent: Arc<Mutex<AgentState>>,
pub url: Url, pub url: Url,
pub is_chunked: bool, pub is_chunked: bool,
pub query_string: String, pub query_string: String,
@@ -239,19 +238,16 @@ pub(crate) fn connect(
} }
#[cfg(feature = "cookie")] #[cfg(feature = "cookie")]
fn extract_cookies(state: &std::sync::Mutex<Option<AgentState>>, url: &Url) -> Option<Header> { fn extract_cookies(state: &std::sync::Mutex<AgentState>, url: &Url) -> Option<Header> {
let state = state.lock().unwrap(); let state = state.lock().unwrap();
let is_secure = url.scheme().eq_ignore_ascii_case("https"); let is_secure = url.scheme().eq_ignore_ascii_case("https");
let hostname = url.host_str().unwrap_or(DEFAULT_HOST).to_string(); let hostname = url.host_str().unwrap_or(DEFAULT_HOST).to_string();
state match_cookies(&state.jar, &hostname, url.path(), is_secure)
.as_ref()
.map(|state| &state.jar)
.and_then(|jar| match_cookies(jar, &hostname, url.path(), is_secure))
} }
#[cfg(not(feature = "cookie"))] #[cfg(not(feature = "cookie"))]
fn extract_cookies(_state: &std::sync::Mutex<Option<AgentState>>, _url: &Url) -> Option<Header> { fn extract_cookies(_state: &std::sync::Mutex<AgentState>, _url: &Url) -> Option<Header> {
None None
} }
@@ -309,15 +305,13 @@ fn connect_socket(unit: &Unit, use_pooled: bool) -> Result<(Stream, bool), Error
}; };
if use_pooled { if use_pooled {
let state = &mut unit.agent.lock().unwrap(); let state = &mut unit.agent.lock().unwrap();
if let Some(agent) = state.as_mut() { // The connection may have been closed by the server
// The connection may have been closed by the server // due to idle timeout while it was sitting in the pool.
// due to idle timeout while it was sitting in the pool. // Loop until we find one that is still good or run out of connections.
// Loop until we find one that is still good or run out of connections. while let Some(stream) = state.pool.try_get_connection(&unit.url, &unit.proxy) {
while let Some(stream) = agent.pool.try_get_connection(&unit.url, &unit.proxy) { let server_closed = stream.server_closed()?;
let server_closed = stream.server_closed()?; if !server_closed {
if !server_closed { return Ok((stream, true));
return Ok((stream, true));
}
} }
} }
} }
@@ -406,20 +400,18 @@ fn save_cookies(unit: &Unit, resp: &Response) {
// only lock if we know there is something to process // only lock if we know there is something to process
let state = &mut unit.agent.lock().unwrap(); let state = &mut unit.agent.lock().unwrap();
if let Some(add_jar) = state.as_mut().map(|state| &mut state.jar) { for raw_cookie in cookies.iter() {
for raw_cookie in cookies.iter() { let to_parse = if raw_cookie.to_lowercase().contains("domain=") {
let to_parse = if raw_cookie.to_lowercase().contains("domain=") { (*raw_cookie).to_string()
(*raw_cookie).to_string() } else {
} else { let host = &unit.url.host_str().unwrap_or(DEFAULT_HOST).to_string();
let host = &unit.url.host_str().unwrap_or(DEFAULT_HOST).to_string(); format!("{}; Domain={}", raw_cookie, host)
format!("{}; Domain={}", raw_cookie, host) };
}; match Cookie::parse_encoded(&to_parse[..]) {
match Cookie::parse_encoded(&to_parse[..]) { Err(_) => (), // ignore unparseable cookies
Err(_) => (), // ignore unparseable cookies Ok(cookie) => {
Ok(cookie) => { let cookie = cookie.into_owned();
let cookie = cookie.into_owned(); state.jar.add(cookie)
add_jar.add(cookie)
}
} }
} }
} }