Merge pull request #30 from mercadopago/feature/add-httpclient-unit-test

Feature/add httpclient unit test

.testcoverage.yml

@@ -7,5 +7,4 @@ threshold:
 exclude:
   paths:
     - ^pkg/internal
-    - ^pkg/config
-    - ^pkg/cardtoken/mock.go
+    - ^pkg/config

pkg/internal/requester/mock.go

@@ -0,0 +1,82 @@
+package requester
+
+import (
+	"bytes"
+	"context"
+	"fmt"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"time"
+)
+
+func NewRequestMock() *http.Request {
+	req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, "", nil)
+
+	return req
+}
+
+func NewRequestMockWithBody() *http.Request {
+	req, _ := http.NewRequest(http.MethodPost, "http://test", bytes.NewBuffer([]byte(`{id:1}`)))
+
+	return req
+}
+
+func NewInvalidRequestMock() *http.Request {
+	req, _ := http.NewRequest(http.MethodGet, "http://test", nil)
+	req.GetBody = func() (io.ReadCloser, error) {
+		return nil, fmt.Errorf("error getting body")
+	}
+
+	return req
+}
+
+func NewRequestMockWithCanceledContext() *http.Request {
+	ctx, cancel := context.WithTimeout(context.Background(), time.Second*1)
+	defer cancel()
+
+	req, _ := http.NewRequestWithContext(ctx, http.MethodGet, "http://test", nil)
+
+	return req
+}
+
+func NewRequestMockWithDeadlineContextAndServerError() (*http.Request, context.CancelFunc) {
+	ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(time.Second*7))
+	req, _ := http.NewRequestWithContext(ctx, http.MethodGet, "", nil)
+
+	return req, cancel
+}
+
+func NewRequestWithHTTPServerUnavailableMock() (*httptest.Server, *http.Request) {
+	s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusServiceUnavailable)
+	}))
+
+	request, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, s.URL, nil)
+
+	return s, request
+}
+
+func NewRequestWithHTTPServerOKMock() (*httptest.Server, *http.Request) {
+	s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+		// we should make this to pass in the lint pileline
+		_, _ = http.ResponseWriter.Write(w, []byte(`{id:1}`))
+	}))
+
+	request, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, s.URL, nil)
+
+	return s, request
+}
+
+func NewRequestWithHTTPServerUnavailableAndCanceledContext() (*httptest.Server, *http.Request, context.CancelFunc) {
+	ctx, cancel := context.WithTimeout(context.Background(), time.Second*1)
+	s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusServiceUnavailable)
+		_, _ = http.ResponseWriter.Write(w, []byte(`{error}`))
+	}))
+
+	request, _ := http.NewRequestWithContext(ctx, http.MethodGet, s.URL, nil)
+
+	return s, request, cancel
+}

pkg/internal/requester/requester.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"io"
 	"net/http"
-	"strconv"
 	"time"
 )
 
@@ -47,7 +46,7 @@ func (d *defaultRequester) Do(req *http.Request) (*http.Response, error) {
 	var err error
 
 	for i := 0; ; i++ {
-		req, err = requestFromInternal(req, i)
+		req, err = requestFromInternal(req)
 		if err != nil {
 			return nil, err
 		}
@@ -82,7 +81,7 @@ func (d *defaultRequester) Do(req *http.Request) (*http.Response, error) {
 		}
 
 		// Call backoff to see how much time we must wait until next retry.
-		backoffWait := backoffDuration(i, resp)
+		backoffWait := backoffDuration(i)
 
 		// If the request context has a deadline, check whether that deadline
 		// happens before the wait period of the backoff strategy. In case
@@ -104,7 +103,7 @@ func (d *defaultRequester) Do(req *http.Request) (*http.Response, error) {
 }
 
 // requestFromInternal builds an *http.Request from our internal request.
-func requestFromInternal(req *http.Request, retryAttempt int) (*http.Request, error) {
+func requestFromInternal(req *http.Request) (*http.Request, error) {
 	ctx := req.Context()
 
 	// Use the context from the internal request. When cloning requests
@@ -158,38 +157,10 @@ func drainBody(body io.ReadCloser) {
 	_, _ = io.Copy(io.Discard, io.LimitReader(body, respReadLimit))
 }
 
-func backoffDuration(attemptNum int, resp *http.Response) time.Duration {
-	if resp != nil {
-		if resp.StatusCode == http.StatusTooManyRequests || resp.StatusCode == http.StatusServiceUnavailable {
-			if s, ok := resp.Header["Retry-After"]; ok {
-				if sleep, err := retryAfterDuration(s[0]); err == nil {
-					return sleep
-				}
-			}
-		}
-	}
-
+func backoffDuration(attemptNum int) time.Duration {
 	return defaultBackoffStrategy(attemptNum)
 }
 
-// retryAfterDuration returns the duration for the Retry-After header.
-func retryAfterDuration(t string) (time.Duration, error) {
-	when, err := time.Parse(http.TimeFormat, t)
-	if err == nil {
-		// when is always in UTC, so make the math from UTC+0.
-		t := time.Now().UTC()
-		return when.Sub(t), nil
-	}
-
-	// The duration can be in seconds.
-	d, err := strconv.Atoi(t)
-	if err != nil {
-		return 0, err
-	}
-
-	return time.Duration(d) * time.Second, nil
-}
-
 // constantBackoff provides a callback for backoffStrategy which will perform
 // linear backoff based on the provided minimum duration.
 func constantBackoff(wait time.Duration) backoffFunc {

pkg/internal/requester/requester_test.go

@@ -0,0 +1,211 @@
+package requester
+
+import (
+	"io"
+	"net/http"
+	"reflect"
+	"testing"
+)
+
+func TestDo(t *testing.T) {
+	server, req := NewRequestWithHTTPServerUnavailableMock()
+	defer server.Close()
+
+	serverOK, reqOK := NewRequestWithHTTPServerOKMock()
+	defer serverOK.Close()
+
+	reqWithDeadline, cancel := NewRequestMockWithDeadlineContextAndServerError()
+	defer cancel()
+
+	type args struct {
+		req *http.Request
+	}
+	tests := []struct {
+		name       string
+		args       args
+		wantStatus string
+		wantErr    string
+	}{
+		{
+			name: "should_return_response_ok_when_status_code_is_200",
+			args: args{
+				req: reqOK,
+			},
+			wantStatus: "200 OK",
+		},
+		{
+			name: "should_retry_and_return_response_error_when_status_code_is_503",
+			args: args{
+				req: req,
+			},
+			wantStatus: "503 Service Unavailable",
+		},
+		{
+			name: "should_return_error_when_context_is_canceled",
+			args: args{
+				req: NewRequestMockWithCanceledContext(),
+			},
+			wantErr: "context canceled",
+		},
+		{
+			name: "should_return_error_when_context_has_deadline_smaller_than_backoff",
+			args: args{
+				req: reqWithDeadline,
+			},
+			wantErr: "Get \"\": unsupported protocol scheme \"\"",
+		},
+		{
+			name: "should_return_error_when_retry_is_enabled_and_request_fails",
+			args: args{
+				req: NewRequestMock(),
+			},
+			wantErr: "Get \"\": unsupported protocol scheme \"\"",
+		},
+		{
+			name: "should_return_error_when_request_is_nil",
+			args: args{
+				req: NewInvalidRequestMock(),
+			},
+			wantErr: "error getting body",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			d := Default()
+			got, err := d.Do(tt.args.req)
+
+			gotError := ""
+			if err != nil {
+				gotError = err.Error()
+			}
+			if gotError != tt.wantErr {
+				t.Errorf("requester.Do() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+
+			status := ""
+			if got != nil {
+				status = got.Status
+			}
+
+			if !reflect.DeepEqual(status, tt.wantStatus) {
+				t.Errorf("requester.Do() = %v, wantStatus %v", status, tt.wantStatus)
+			}
+		})
+	}
+}
+
+func TestRequestFromInternal(t *testing.T) {
+	type args struct {
+		req *http.Request
+	}
+	tests := []struct {
+		name    string
+		args    args
+		want    string
+		wantErr string
+	}{
+		{
+			name: "should_copy_and_return_request_with_body",
+			args: args{
+				req: NewRequestMockWithBody(),
+			},
+			want: "{id:1}",
+		},
+		{
+			name: "should_copy_and_return_request_with_body_nil",
+			args: args{
+				req: NewRequestMock(),
+			},
+			want: "",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := requestFromInternal(tt.args.req)
+			gotError := ""
+			if err != nil {
+				gotError = err.Error()
+			}
+
+			if gotError != tt.wantErr {
+				t.Errorf("requester.requestFromInternal() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+
+			body := ""
+			if got.Body != nil {
+				bytes, _ := io.ReadAll(got.Body)
+				body = string(bytes)
+			}
+
+			if tt.want != body {
+				t.Errorf("requester.requestFromInternal() = %v, want %v", body, tt.want)
+			}
+		})
+	}
+}
+
+func TestCloseResponseBody(t *testing.T) {
+	server, req := NewRequestWithHTTPServerOKMock()
+	defer server.Close()
+
+	s, reqWithResAndCancel, cancel := NewRequestWithHTTPServerUnavailableAndCanceledContext()
+	defer s.Close()
+	defer cancel()
+
+	type args struct {
+		req   *http.Request
+		close func(*http.Response)
+	}
+	tests := []struct {
+		name    string
+		args    args
+		wantErr string
+	}{
+		{
+			name: "should_close_body_after_read",
+			args: args{
+				req: req,
+				close: func(r *http.Response) {
+					r.Body.Close()
+				},
+			},
+			wantErr: "http: read on closed response body",
+		},
+		{
+			name: "should_not_close_body_after_read",
+			args: args{
+				req:   req,
+				close: func(_ *http.Response) {},
+			},
+			wantErr: "",
+		},
+		{
+			name: "should_close_body_when_response_has_error_and_context_is_canceled",
+			args: args{
+				req:   reqWithResAndCancel,
+				close: func(_ *http.Response) {},
+			},
+			wantErr: "http: read on closed response body",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			d := Default()
+			got, _ := d.Do(tt.args.req)
+
+			tt.args.close(got)
+
+			_, err := io.ReadAll(got.Body)
+			gotError := ""
+			if err != nil {
+				gotError = err.Error()
+			}
+
+			if !reflect.DeepEqual(gotError, tt.wantErr) {
+				t.Errorf("requester.Do() error = %v, wantError %v", err, tt.wantErr)
+			}
+		})
+	}
+}