lib/http: refactoring NewClient to accept single struct

Previously, the NewClient function accept three parameters: serverURL,
http.Header, and insecure.  If we want to add another parameter,
for example timeout it will cause changes on the function signature.

To prevent this changes in the future, we change it now. The NewClient
now accept single struct.

While at it, we add option to set Timeout.

The Timeout affect the http Transport Timeout and TLSHandshakeTimeout.
The field is optional, if not set it will set to 10 seconds.

8 files changed, 92 insertions, 51 deletions

diff --git a/api/slack/webhook_client.go b/api/slack/webhook_client.go
index 1896f9fc..e5ac2f45 100644
--- a/api/slack/webhook_client.go
+++ b/api/slack/webhook_client.go
@@ -35,9 +35,11 @@ func NewWebhookClient(webhookURL, user, channel string) (wcl *WebhookClient, err
 		return nil, fmt.Errorf("NewWebhookClient: %w", err)
 	}
 
-	host := fmt.Sprintf("%s://%s", wurl.Scheme, wurl.Host)
+	clientOpts := &libhttp.ClientOptions{
+		ServerUrl: fmt.Sprintf("%s://%s", wurl.Scheme, wurl.Host),
+	}
 	wcl = &WebhookClient{
-		Client:      libhttp.NewClient(host, nil, false),
+		Client:      libhttp.NewClient(clientOpts),
 		webhookPath: wurl.Path,
 		user:        user,
 		channel:     channel,
diff --git a/api/telegram/bot/bot.go b/api/telegram/bot/bot.go
index 844a8c25..1a5eeb97 100644
--- a/api/telegram/bot/bot.go
+++ b/api/telegram/bot/bot.go
@@ -114,10 +114,12 @@ func New(opts Options) (bot *Bot, err error) {
 		return nil, fmt.Errorf("bot.New: %w", err)
 	}
 
-	serverURL := defURL + opts.Token + "/"
+	clientOpts := &http.ClientOptions{
+		ServerUrl: defURL + opts.Token + "/",
+	}
 	bot = &Bot{
 		opts:   opts,
-		client: http.NewClient(serverURL, nil, false),
+		client: http.NewClient(clientOpts),
 	}
 
 	fmt.Printf("Bot options: %+v\n", opts)
diff --git a/lib/http/client.go b/lib/http/client.go
index 16ab3fe7..b141f31b 100644
--- a/lib/http/client.go
+++ b/lib/http/client.go
@@ -42,49 +42,39 @@ type Client struct {
 	flateReader io.ReadCloser
 	gzipReader  *gzip.Reader
 
-	defHeaders http.Header
-	*http.Client
+	opts *ClientOptions
 
-	serverURL string
+	*http.Client
 }
 
 //
-// NewClient create and initialize new Client connection using serverURL to
-// minimize repetition.
-// The serverURL is any path that is static and will never changes during
-// request to server.
-// The headers parameter define default headers that will be set in any
-// request to server.
-// The insecure parameter allow to connect to remote server with unknown
-// certificate authority.
+// NewClient create and initialize new Client.
 //
-func NewClient(serverURL string, headers http.Header, insecure bool) (client *Client) {
-	if headers == nil {
-		headers = make(http.Header)
-	}
+// The client will have KeepAlive timeout set to 30 seconds, with 1 maximum
+// idle connection, and 90 seconds IdleConnTimeout.
+//
+func NewClient(opts *ClientOptions) (client *Client) {
+	opts.init()
 
 	httpTransport := &http.Transport{
 		Proxy: http.ProxyFromEnvironment,
 		DialContext: (&net.Dialer{
-			Timeout:   30 * time.Second,
+			Timeout:   opts.Timeout,
 			KeepAlive: 30 * time.Second,
-			DualStack: true,
 		}).DialContext,
-		ForceAttemptHTTP2:     true,
-		MaxIdleConns:          100,
+		MaxIdleConns:          1,
 		IdleConnTimeout:       90 * time.Second,
-		TLSHandshakeTimeout:   10 * time.Second,
+		TLSHandshakeTimeout:   opts.Timeout,
 		ExpectContinueTimeout: 1 * time.Second,
 	}
 
 	client = &Client{
-		serverURL:  serverURL,
-		defHeaders: headers,
-		Client:     &http.Client{},
+		opts:   opts,
+		Client: &http.Client{},
 	}
-	if insecure {
+	if opts.AllowInsecure {
 		httpTransport.TLSClientConfig = &tls.Config{
-			InsecureSkipVerify: insecure,
+			InsecureSkipVerify: opts.AllowInsecure,
 		}
 	}
 	client.Client.Transport = httpTransport
@@ -254,14 +244,14 @@ func (client *Client) GenerateHttpRequest(
 		}
 	}
 
-	fullURL := client.serverURL + path
+	fullURL := client.opts.ServerUrl + path
 
 	httpRequest, err = http.NewRequest(method.String(), fullURL, body)
 	if err != nil {
 		return nil, fmt.Errorf("%s: %w", logp, err)
 	}
 
-	client.setHeaders(httpRequest, client.defHeaders)
+	client.setHeaders(httpRequest, client.opts.Headers)
 	client.setHeaders(httpRequest, headers)
 
 	if len(contentType) > 0 {
@@ -386,14 +376,14 @@ func (client *Client) doRequest(
 ) (
 	httpRes *http.Response, resBody []byte, err error,
 ) {
-	fullURL := client.serverURL + path
+	fullURL := client.opts.ServerUrl + path
 
 	httpReq, err := http.NewRequest(httpMethod, fullURL, body)
 	if err != nil {
 		return nil, nil, err
 	}
 
-	client.setHeaders(httpReq, client.defHeaders)
+	client.setHeaders(httpReq, client.opts.Headers)
 	client.setHeaders(httpReq, headers)
 
 	if len(contentType) > 0 {
@@ -422,11 +412,11 @@ func (client *Client) setHeaders(req *http.Request, headers http.Header) {
 // setUserAgent set the User-Agent header only if its not defined by user.
 //
 func (client *Client) setUserAgent() {
-	v := client.defHeaders.Get(HeaderUserAgent)
+	v := client.opts.Headers.Get(HeaderUserAgent)
 	if len(v) > 0 {
 		return
 	}
-	client.defHeaders.Set(HeaderUserAgent, defUserAgent)
+	client.opts.Headers.Set(HeaderUserAgent, defUserAgent)
 }
 
 //
diff --git a/lib/http/client_options.go b/lib/http/client_options.go
new file mode 100644
index 00000000..129a239b
--- /dev/null
+++ b/lib/http/client_options.go
@@ -0,0 +1,43 @@
+package http
+
+import (
+	"net/http"
+	"time"
+)
+
+const (
+	defClientTimeout = 10 * time.Second
+)
+
+//
+// ClientOptions options for HTTP client.
+//
+type ClientOptions struct {
+	// Headers define default headers that will be send in any request to
+	// server.
+	// This field is optional.
+	Headers http.Header
+
+	// ServerUrl is any path that is static and will never changes during
+	// request to server.
+	// This field is required.
+	ServerUrl string
+
+	// Timeout affect the http Transport Timeout and TLSHandshakeTimeout.
+	// This field is optional, if not set it will set to 10 seconds.
+	Timeout time.Duration
+
+	// AllowInsecure if its true, it will allow to connect to server with
+	// unknown certificate authority.
+	// This field is optional.
+	AllowInsecure bool
+}
+
+func (opts *ClientOptions) init() {
+	if opts.Headers == nil {
+		opts.Headers = make(http.Header)
+	}
+	if opts.Timeout <= 0 {
+		opts.Timeout = defClientTimeout
+	}
+}
diff --git a/lib/http/endpoint_example_test.go b/lib/http/endpoint_example_test.go
index 55b11c64..e5e8238b 100644
--- a/lib/http/endpoint_example_test.go
+++ b/lib/http/endpoint_example_test.go
@@ -51,7 +51,10 @@ func ExampleEndpoint_errorHandler() {
 	}()
 	time.Sleep(1 * time.Second)
 
-	client := NewClient("http://"+serverOpts.Address, nil, false)
+	clientOpts := &ClientOptions{
+		ServerUrl: "http://" + serverOpts.Address,
+	}
+	client := NewClient(clientOpts)
 
 	params := url.Values{}
 	params.Set("error", "400:error with status code")
diff --git a/lib/http/endpoint_response_example_test.go b/lib/http/endpoint_response_example_test.go
index 6f98dc61..5e953f1b 100644
--- a/lib/http/endpoint_response_example_test.go
+++ b/lib/http/endpoint_response_example_test.go
@@ -67,7 +67,10 @@ func ExampleEndpointResponse() {
 	}()
 	time.Sleep(500)
 
-	cl := NewClient("http://127.0.0.1:7016", nil, true)
+	clientOpts := &ClientOptions{
+		ServerUrl: "http://127.0.0.1:7016",
+	}
+	cl := NewClient(clientOpts)
 	params := url.Values{}
 
 	// Test call endpoint without "id" parameter.
diff --git a/lib/http/example_server_test.go b/lib/http/example_server_test.go
index 2c886f27..aad952aa 100644
--- a/lib/http/example_server_test.go
+++ b/lib/http/example_server_test.go
@@ -60,7 +60,10 @@ func ExampleServer_customHTTPStatusCode() {
 	// Wait for the server fully started.
 	time.Sleep(1 * time.Second)
 
-	client := NewClient("http://127.0.0.1:8123", nil, false)
+	clientOpts := &ClientOptions{
+		ServerUrl: "http://127.0.0.1:8123",
+	}
+	client := NewClient(clientOpts)
 
 	httpRes, resBody, err := client.PostJSON(epCustom.Path, nil, nil)
 	if err != nil {
diff --git a/lib/xmlrpc/client.go b/lib/xmlrpc/client.go
index dfe4e016..367e8849 100644
--- a/lib/xmlrpc/client.go
+++ b/lib/xmlrpc/client.go
@@ -46,32 +46,27 @@ func NewClient(url *url.URL, timeout time.Duration) (client *Client, err error)
 		url:     url,
 		timeout: timeout,
 	}
+	clientOpts := &libhttp.ClientOptions{
+		Timeout: timeout,
+	}
 
 	if url.Scheme == schemeIsHTTPS {
-		var insecure bool
 		if ip != nil {
-			insecure = true
+			clientOpts.AllowInsecure = true
 		}
 		if port == 0 {
 			port = 443
 		}
-
-		serverUrl := fmt.Sprintf("https://%s:%d", host, port)
-
-		client.conn = libhttp.NewClient(serverUrl, nil, insecure)
+		clientOpts.ServerUrl = fmt.Sprintf("https://%s:%d", host, port)
 	} else {
 		if port == 0 {
 			port = 80
 		}
-
-		serverUrl := fmt.Sprintf("http://%s:%d", host, port)
-
-		client.conn = libhttp.NewClient(serverUrl, nil, false)
-	}
-	if err != nil {
-		return nil, fmt.Errorf("NewClient: Dial: %w", err)
+		clientOpts.ServerUrl = fmt.Sprintf("http://%s:%d", host, port)
 	}
 
+	client.conn = libhttp.NewClient(clientOpts)
+
 	return client, nil
 }