refactor awc::ClientBuilder (#2008)

actix-http-test/src/lib.rs

@@ -96,14 +96,12 @@ pub async fn test_server_with_addr<F: ServiceFactory<TcpStream>>(
                     .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()
             }
         };
 

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};

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<T, U>` 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

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<T = (), U = ()> {
     default_headers: bool,
-    allow_redirects: bool,
-    max_redirects: usize,
     max_http_version: Option<http::Version>,
     stream_window_size: Option<u32>,
     conn_window_size: Option<u32>,
     headers: HeaderMap,
     timeout: Option<Duration>,
-    connector: Option<ConnectService>,
-}
-
-impl Default for ClientBuilder {
-    fn default() -> Self {
-        Self::new()
-    }
+    connector: Connector<T, U>,
 }
 
 impl ClientBuilder {
-    pub fn new() -> Self {
+    #[allow(clippy::new_ret_no_self)]
+    pub fn new() -> ClientBuilder<
+        impl Service<
+                TcpConnect<Uri>,
+                Response = TcpConnection<Uri, TcpStream>,
+                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<S, Io> ClientBuilder<S, Io>
+where
+    S: Service<TcpConnect<Uri>, Response = TcpConnection<Uri, Io>, Error = TcpConnectError>
+        + Clone
+        + 'static,
+    Io: AsyncRead + AsyncWrite + Unpin + fmt::Debug + 'static,
+{
     /// Use custom connector service.
-    pub fn connector<T>(mut self, connector: T) -> Self
+    pub fn connector<S1, Io1>(self, connector: Connector<S1, Io1>) -> ClientBuilder<S1, Io1>
     where
-        T: Service<HttpConnect, Error = ConnectError> + 'static,
-        T::Response: Connection,
-        T::Future: 'static,
+        S1: Service<
+                TcpConnect<Uri>,
+                Response = TcpConnection<Uri, Io1>,
+                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<U>(self, username: U, password: Option<&str>) -> Self
+    pub fn basic_auth<N>(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))
     }
 }

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<Uri>,
+                Response = TcpConnection<Uri, TcpStream>,
+                Error = TcpConnectError,
+            > + Clone,
+        TcpStream,
+    > {
         ClientBuilder::new()
     }
 

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

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();

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

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

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()
             }
         };
 

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();