diff --git a/.testcoverage.yml b/.testcoverage.yml index 1db2c06b..78407e6f 100644 --- a/.testcoverage.yml +++ b/.testcoverage.yml @@ -7,5 +7,4 @@ threshold: exclude: paths: - ^pkg/internal - - ^pkg/config - - ^pkg/cardtoken/mock.go \ No newline at end of file + - ^pkg/config \ No newline at end of file diff --git a/pkg/internal/requester/mock.go b/pkg/internal/requester/mock.go new file mode 100644 index 00000000..76485273 --- /dev/null +++ b/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 +} diff --git a/pkg/internal/requester/requester.go b/pkg/internal/requester/requester.go index 4f0f3d69..aedfa7f1 100644 --- a/pkg/internal/requester/requester.go +++ b/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 { diff --git a/pkg/internal/requester/requester_test.go b/pkg/internal/requester/requester_test.go new file mode 100644 index 00000000..05f02279 --- /dev/null +++ b/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) + } + }) + } +}