Clean HTTP client fields.

Delete `redirect_count`.
Make `handle private.

packages/hurl/src/http/client.rs

@@ -37,13 +37,14 @@ use curl::easy::{List, SslOpt};
 use std::str::FromStr;
 use url::Url;
 
+/// Defines an HTTP client to execute HTTP requests.
+///
+/// Most of the methods are delegated to libcurl functions, while some
+/// features are implemented "by hand" (like retry, redirection etc...)
 #[derive(Debug)]
 pub struct Client {
-    pub handle: Box<easy::Easy>,
+    /// The handle to libcurl binding
-    pub redirect_count: usize,
+    handle: Box<easy::Easy>,
-    // Unfortunately, follow-location feature from libcurl can not be used
-    // libcurl returns a single list of headers for the 2 responses
-    // Hurl needs to keep everything.
 }
 
 impl Client {
@@ -61,7 +62,6 @@ impl Client {
 
         Client {
             handle: Box::new(h),
-            redirect_count: 0,
         }
     }
 
@@ -76,7 +76,11 @@ impl Client {
         let mut calls = vec![];
 
         let mut request_spec = request_spec.clone();
-        self.redirect_count = 0;
+
+        // Unfortunately, follow-location feature from libcurl can not be used
+        // libcurl returns a single list of headers for the 2 responses
+        // Hurl needs to keep everything.
+        let mut redirect_count = 0;
         loop {
             let (request, response) = self.execute(&request_spec, options, logger)?;
             calls.push((request.clone(), response.clone()));
@@ -101,9 +105,9 @@ impl Client {
                     content_type: None,
                 };
 
-                self.redirect_count += 1;
+                redirect_count += 1;
                 if let Some(max_redirect) = options.max_redirect {
-                    if self.redirect_count > max_redirect {
+                    if redirect_count > max_redirect {
                         return Err(HttpError::TooManyRedirect);
                     }
                 }

packages/hurl/tests/libcurl.rs

@@ -22,7 +22,7 @@ use std::time::Duration;
 use hurl::http::*;
 use hurl::util::logger::LoggerBuilder;
 
-pub fn new_header(name: &str, value: &str) -> Header {
+fn new_header(name: &str, value: &str) -> Header {
     Header {
         name: name.to_string(),
         value: value.to_string(),
@@ -43,7 +43,12 @@ fn default_get_request(url: &str) -> RequestSpec {
     }
 }
 
-// region basic
+fn redirect_count(calls: &[(Request, Response)]) -> usize {
+    calls
+        .iter()
+        .filter(|(_, resp)| resp.status >= 300 && resp.status < 399)
+        .count()
+}
 
 #[test]
 fn test_hello() {
@@ -88,10 +93,6 @@ fn test_hello() {
     assert_eq!(response.get_header_values("Date").len(), 1);
 }
 
-// endregion
-
-// region http method
-
 #[test]
 fn test_put() {
     let options = ClientOptions::default();
@@ -186,10 +187,6 @@ fn test_patch() {
     assert!(response.body.is_empty());
 }
 
-// endregion
-
-// region headers
-
 #[test]
 fn test_custom_headers() {
     let options = ClientOptions::default();
@@ -235,10 +232,6 @@ fn test_custom_headers() {
     assert!(response.body.is_empty());
 }
 
-// endregion
-
-// region querystrings
-
 #[test]
 fn test_querystring_params() {
     let options = ClientOptions::default();
@@ -292,10 +285,6 @@ fn test_querystring_params() {
     assert!(response.body.is_empty());
 }
 
-// endregion
-
-// region form params
-
 #[test]
 fn test_form_params() {
     let options = ClientOptions::default();
@@ -366,10 +355,6 @@ fn test_form_params() {
     assert_eq!(response.body, b"Hello World!".to_vec());
 }
 
-// endregion
-
-// region redirect
-
 #[test]
 fn test_redirect() {
     let request_spec = default_get_request("http://localhost:8000/redirect-absolute");
@@ -395,7 +380,6 @@ fn test_redirect() {
         response.url,
         "http://localhost:8000/redirect-absolute".to_string()
     );
-    assert_eq!(client.redirect_count, 0);
 }
 
 #[test]
@@ -448,17 +432,14 @@ fn test_follow_location() {
         "http://localhost:8000/redirected".to_string()
     );
 
-    assert_eq!(client.redirect_count, 1);
-
-    // make sure that the redirect count is reset to 0
     let request = default_get_request("http://localhost:8000/hello");
     let calls = client
         .execute_with_redirect(&request, &options, &logger)
         .unwrap();
+    assert_eq!(calls.len(), 1);
     let (_, response) = calls.get(0).unwrap();
     assert_eq!(response.status, 200);
     assert_eq!(response.body, b"Hello World!".to_vec());
-    assert_eq!(client.redirect_count, 0);
 }
 
 #[test]
@@ -495,13 +476,9 @@ fn test_max_redirect() {
     let (request, response) = calls.last().unwrap();
     assert_eq!(request.url, "http://localhost:8000/redirect/0".to_string());
     assert_eq!(response.status, 200);
-    assert_eq!(client.redirect_count, 8);
+    assert_eq!(redirect_count(&calls), 8);
 }
 
-// endregion
-
-// region multipart
-
 #[test]
 fn test_multipart_form_data() {
     let options = ClientOptions::default();
@@ -565,10 +542,6 @@ fn test_multipart_form_data() {
     assert_eq!(response.body, b"Hello World!".to_vec());
 }
 
-// endregion
-
-// region http body
-
 #[test]
 fn test_post_bytes() {
     let options = ClientOptions::default();
@@ -602,8 +575,6 @@ fn test_post_bytes() {
     assert!(response.body.is_empty());
 }
 
-// endregion
-
 #[test]
 fn test_expect() {
     let options = ClientOptions::default();
@@ -719,8 +690,6 @@ fn test_cacert() {
     assert_eq!(response.status, 200);
 }
 
-// region error
-
 #[test]
 fn test_error_could_not_resolve_host() {
     let options = ClientOptions::default();
@@ -964,9 +933,6 @@ fn test_connect_timeout() {
         assert_eq!(url, "http://10.0.0.0");
     }
 }
-// endregion
-
-// region cookie
 
 #[test]
 fn test_cookie() {
@@ -1142,10 +1108,6 @@ fn test_cookie_file() {
     assert!(response.body.is_empty());
 }
 
-// endregion
-
-// region proxy
-
 #[test]
 fn test_proxy() {
     // mitmproxy listening on port 8888
@@ -1168,8 +1130,6 @@ fn test_proxy() {
     assert_eq!(response.status, 200);
 }
 
-// endregion
-
 #[test]
 fn test_insecure() {
     let options = ClientOptions {