From 946cccaa1a2ace8d997723399da4d5e0b43457e3 Mon Sep 17 00:00:00 2001 From: fakeshadow <24548779@qq.com> Date: Thu, 18 Feb 2021 04:30:09 -0800 Subject: [PATCH] refactor awc::ClientBuilder (#2008) --- actix-http-test/src/lib.rs | 2 - actix-http/src/client/mod.rs | 4 ++ awc/CHANGES.md | 7 +- awc/src/builder.rs | 115 ++++++++++++++++---------------- awc/src/lib.rs | 18 ++++- awc/tests/test_client.rs | 7 +- awc/tests/test_connector.rs | 2 +- awc/tests/test_rustls_client.rs | 2 +- awc/tests/test_ssl_client.rs | 2 +- src/test.rs | 2 - tests/test_httpserver.rs | 3 +- 11 files changed, 91 insertions(+), 73 deletions(-) diff --git a/actix-http-test/src/lib.rs b/actix-http-test/src/lib.rs index df5774998..8de07c8d3 100644 --- a/actix-http-test/src/lib.rs +++ b/actix-http-test/src/lib.rs @@ -96,14 +96,12 @@ pub async fn test_server_with_addr>( .conn_lifetime(time::Duration::from_secs(0)) .timeout(time::Duration::from_millis(30000)) .ssl(builder.build()) - .finish() } #[cfg(not(feature = "openssl"))] { Connector::new() .conn_lifetime(time::Duration::from_secs(0)) .timeout(time::Duration::from_millis(30000)) - .finish() } }; diff --git a/actix-http/src/client/mod.rs b/actix-http/src/client/mod.rs index 9c7f632ea..5f5e57edb 100644 --- a/actix-http/src/client/mod.rs +++ b/actix-http/src/client/mod.rs @@ -10,6 +10,10 @@ mod h1proto; mod h2proto; mod pool; +pub use actix_tls::connect::{ + Connect as TcpConnect, ConnectError as TcpConnectError, Connection as TcpConnection, +}; + pub use self::connection::Connection; pub use self::connector::Connector; pub use self::error::{ConnectError, FreezeRequestError, InvalidUrl, SendRequestError}; diff --git a/awc/CHANGES.md b/awc/CHANGES.md index c67f65560..e6ead2cc8 100644 --- a/awc/CHANGES.md +++ b/awc/CHANGES.md @@ -6,10 +6,15 @@ ### Changed * Feature `cookies` is now optional and enabled by default. [#1981] +* `ClientBuilder::connector` method would take `actix_http::client::Connector` type. [#2008] + +### Removed +* `ClientBuilder::default` function [#2008] +* `ClientBuilder::disable_redirects` and `ClientBuilder::max_redirects` method [#2008] [#1931]: https://github.com/actix/actix-web/pull/1931 [#1981]: https://github.com/actix/actix-web/pull/1981 - +[#2008]: https://github.com/actix/actix-web/pull/2008 ## 3.0.0-beta.2 - 2021-02-10 ### Added diff --git a/awc/src/builder.rs b/awc/src/builder.rs index b5e8bf5bf..4495b39fd 100644 --- a/awc/src/builder.rs +++ b/awc/src/builder.rs @@ -3,59 +3,80 @@ use std::fmt; use std::rc::Rc; use std::time::Duration; -use actix_http::client::{Connect as HttpConnect, ConnectError, Connection, Connector}; -use actix_http::http::{self, header, Error as HttpError, HeaderMap, HeaderName}; +use actix_codec::{AsyncRead, AsyncWrite}; +use actix_http::{ + client::{Connector, TcpConnect, TcpConnectError, TcpConnection}, + http::{self, header, Error as HttpError, HeaderMap, HeaderName, Uri}, +}; +use actix_rt::net::TcpStream; use actix_service::Service; -use crate::connect::{ConnectService, ConnectorWrapper}; +use crate::connect::ConnectorWrapper; use crate::{Client, ClientConfig}; /// An HTTP Client builder /// /// This type can be used to construct an instance of `Client` through a /// builder-like pattern. -pub struct ClientBuilder { +pub struct ClientBuilder { default_headers: bool, - allow_redirects: bool, - max_redirects: usize, max_http_version: Option, stream_window_size: Option, conn_window_size: Option, headers: HeaderMap, timeout: Option, - connector: Option, -} - -impl Default for ClientBuilder { - fn default() -> Self { - Self::new() - } + connector: Connector, } impl ClientBuilder { - pub fn new() -> Self { + #[allow(clippy::new_ret_no_self)] + pub fn new() -> ClientBuilder< + impl Service< + TcpConnect, + Response = TcpConnection, + Error = TcpConnectError, + > + Clone, + TcpStream, + > { ClientBuilder { default_headers: true, - allow_redirects: true, - max_redirects: 10, headers: HeaderMap::new(), timeout: Some(Duration::from_secs(5)), - connector: None, + connector: Connector::new(), max_http_version: None, stream_window_size: None, conn_window_size: None, } } +} +impl ClientBuilder +where + S: Service, Response = TcpConnection, Error = TcpConnectError> + + Clone + + 'static, + Io: AsyncRead + AsyncWrite + Unpin + fmt::Debug + 'static, +{ /// Use custom connector service. - pub fn connector(mut self, connector: T) -> Self + pub fn connector(self, connector: Connector) -> ClientBuilder where - T: Service + 'static, - T::Response: Connection, - T::Future: 'static, + S1: Service< + TcpConnect, + Response = TcpConnection, + Error = TcpConnectError, + > + Clone + + 'static, + Io1: AsyncRead + AsyncWrite + Unpin + fmt::Debug + 'static, { - self.connector = Some(Box::new(ConnectorWrapper::new(connector))); - self + ClientBuilder { + default_headers: self.default_headers, + headers: self.headers, + timeout: self.timeout, + connector, + max_http_version: self.max_http_version, + stream_window_size: self.stream_window_size, + conn_window_size: self.conn_window_size, + } } /// Set request timeout @@ -73,14 +94,6 @@ impl ClientBuilder { self } - /// Do not follow redirects. - /// - /// Redirects are allowed by default. - pub fn disable_redirects(mut self) -> Self { - self.allow_redirects = false; - self - } - /// Maximum supported HTTP major version. /// /// Supported versions are HTTP/1.1 and HTTP/2. @@ -107,14 +120,6 @@ impl ClientBuilder { self } - /// Set max number of redirects. - /// - /// Max redirects is set to 10 by default. - pub fn max_redirects(mut self, num: usize) -> Self { - self.max_redirects = num; - self - } - /// Do not add default request headers. /// By default `Date` and `User-Agent` headers are set. pub fn no_default_headers(mut self) -> Self { @@ -144,9 +149,9 @@ impl ClientBuilder { } /// Set client wide HTTP basic authorization header - pub fn basic_auth(self, username: U, password: Option<&str>) -> Self + pub fn basic_auth(self, username: N, password: Option<&str>) -> Self where - U: fmt::Display, + N: fmt::Display, { let auth = match password { Some(password) => format!("{}:{}", username, password), @@ -168,26 +173,24 @@ impl ClientBuilder { /// Finish build process and create `Client` instance. pub fn finish(self) -> Client { - let connector = if let Some(connector) = self.connector { - connector - } else { - let mut connector = Connector::new(); - if let Some(val) = self.max_http_version { - connector = connector.max_http_version(val) - }; - if let Some(val) = self.conn_window_size { - connector = connector.initial_connection_window_size(val) - }; - if let Some(val) = self.stream_window_size { - connector = connector.initial_window_size(val) - }; - Box::new(ConnectorWrapper::new(connector.finish())) as _ + let mut connector = self.connector; + + if let Some(val) = self.max_http_version { + connector = connector.max_http_version(val); }; + if let Some(val) = self.conn_window_size { + connector = connector.initial_connection_window_size(val) + }; + if let Some(val) = self.stream_window_size { + connector = connector.initial_window_size(val) + }; + let config = ClientConfig { headers: self.headers, timeout: self.timeout, - connector, + connector: Box::new(ConnectorWrapper::new(connector.finish())) as _, }; + Client(Rc::new(config)) } } diff --git a/awc/src/lib.rs b/awc/src/lib.rs index 1e27ed9ab..66ff55402 100644 --- a/awc/src/lib.rs +++ b/awc/src/lib.rs @@ -101,8 +101,13 @@ use std::time::Duration; pub use actix_http::cookie; pub use actix_http::{client::Connector, http}; -use actix_http::http::{Error as HttpError, HeaderMap, Method, Uri}; -use actix_http::RequestHead; +use actix_http::{ + client::{TcpConnect, TcpConnectError, TcpConnection}, + http::{Error as HttpError, HeaderMap, Method, Uri}, + RequestHead, +}; +use actix_rt::net::TcpStream; +use actix_service::Service; mod builder; mod connect; @@ -169,7 +174,14 @@ impl Client { /// Create `Client` builder. /// This function is equivalent of `ClientBuilder::new()`. - pub fn builder() -> ClientBuilder { + pub fn builder() -> ClientBuilder< + impl Service< + TcpConnect, + Response = TcpConnection, + Error = TcpConnectError, + > + Clone, + TcpStream, + > { ClientBuilder::new() } diff --git a/awc/tests/test_client.rs b/awc/tests/test_client.rs index bcbaf3f41..a41a8dac3 100644 --- a/awc/tests/test_client.rs +++ b/awc/tests/test_client.rs @@ -120,8 +120,7 @@ async fn test_timeout() { let connector = awc::Connector::new() .connector(actix_tls::connect::default_connector()) - .timeout(Duration::from_secs(15)) - .finish(); + .timeout(Duration::from_secs(15)); let client = awc::Client::builder() .connector(connector) @@ -368,7 +367,7 @@ async fn test_connection_wait_queue() { .await; let client = awc::Client::builder() - .connector(awc::Connector::new().limit(1).finish()) + .connector(awc::Connector::new().limit(1)) .finish(); // req 1 @@ -417,7 +416,7 @@ async fn test_connection_wait_queue_force_close() { .await; let client = awc::Client::builder() - .connector(awc::Connector::new().limit(1).finish()) + .connector(awc::Connector::new().limit(1)) .finish(); // req 1 diff --git a/awc/tests/test_connector.rs b/awc/tests/test_connector.rs index fd725506d..632f68b72 100644 --- a/awc/tests/test_connector.rs +++ b/awc/tests/test_connector.rs @@ -58,7 +58,7 @@ async fn test_connection_window_size() { .map_err(|e| log::error!("Can not set alpn protocol: {:?}", e)); let client = awc::Client::builder() - .connector(awc::Connector::new().ssl(builder.build()).finish()) + .connector(awc::Connector::new().ssl(builder.build())) .initial_window_size(100) .initial_connection_window_size(100) .finish(); diff --git a/awc/tests/test_rustls_client.rs b/awc/tests/test_rustls_client.rs index a928715a8..464edfe89 100644 --- a/awc/tests/test_rustls_client.rs +++ b/awc/tests/test_rustls_client.rs @@ -82,7 +82,7 @@ async fn test_connection_reuse_h2() { .set_certificate_verifier(Arc::new(danger::NoCertificateVerification)); let client = awc::Client::builder() - .connector(awc::Connector::new().rustls(Arc::new(config)).finish()) + .connector(awc::Connector::new().rustls(Arc::new(config))) .finish(); // req 1 diff --git a/awc/tests/test_ssl_client.rs b/awc/tests/test_ssl_client.rs index 08aa125cd..3079aaf5e 100644 --- a/awc/tests/test_ssl_client.rs +++ b/awc/tests/test_ssl_client.rs @@ -72,7 +72,7 @@ async fn test_connection_reuse_h2() { .map_err(|e| log::error!("Can not set alpn protocol: {:?}", e)); let client = awc::Client::builder() - .connector(awc::Connector::new().ssl(builder.build()).finish()) + .connector(awc::Connector::new().ssl(builder.build())) .finish(); // req 1 diff --git a/src/test.rs b/src/test.rs index 2ec4252b1..dd2426fec 100644 --- a/src/test.rs +++ b/src/test.rs @@ -761,14 +761,12 @@ where .conn_lifetime(time::Duration::from_secs(0)) .timeout(time::Duration::from_millis(30000)) .ssl(builder.build()) - .finish() } #[cfg(not(feature = "openssl"))] { Connector::new() .conn_lifetime(time::Duration::from_secs(0)) .timeout(time::Duration::from_millis(30000)) - .finish() } }; diff --git a/tests/test_httpserver.rs b/tests/test_httpserver.rs index 3aa1d36b0..043159376 100644 --- a/tests/test_httpserver.rs +++ b/tests/test_httpserver.rs @@ -137,8 +137,7 @@ async fn test_start_ssl() { .connector( awc::Connector::new() .ssl(builder.build()) - .timeout(Duration::from_millis(100)) - .finish(), + .timeout(Duration::from_millis(100)), ) .finish();