From 02a3b5d17e402573384f65efcc0b32e7863dd8e6 Mon Sep 17 00:00:00 2001 From: Roman Dmytrenko Date: Tue, 27 Aug 2024 13:17:43 +0300 Subject: [PATCH] fix(ofrep): close http response body after using it (#576) --- .../ofrep/internal/evaluate/flags_test.go | 8 ++-- providers/ofrep/internal/outbound/http.go | 1 + .../ofrep/internal/outbound/http_test.go | 32 ++----------- providers/ofrep/provider_test.go | 45 ++++++------------- 4 files changed, 23 insertions(+), 63 deletions(-) diff --git a/providers/ofrep/internal/evaluate/flags_test.go b/providers/ofrep/internal/evaluate/flags_test.go index 95e3dd0f9..03293f496 100644 --- a/providers/ofrep/internal/evaluate/flags_test.go +++ b/providers/ofrep/internal/evaluate/flags_test.go @@ -126,7 +126,7 @@ func TestBooleanEvaluation(t *testing.T) { expect: false, }, { - name: "disbaled flag", + name: "disabled flag", resolver: mockResolver{ success: &successDisabled, }, @@ -192,7 +192,7 @@ func TestIntegerEvaluation(t *testing.T) { expect: 1, }, { - name: "disbaled flag", + name: "disabled flag", resolver: mockResolver{ success: &successDisabled, }, @@ -250,7 +250,7 @@ func TestFloatEvaluation(t *testing.T) { expect: 1.05, }, { - name: "disbaled flag", + name: "disabled flag", resolver: mockResolver{ success: &successDisabled, }, @@ -341,7 +341,7 @@ func TestObjectEvaluation(t *testing.T) { expect: map[string]interface{}{}, }, { - name: "disbaled flag", + name: "disabled flag", resolver: mockResolver{ success: &successDisabled, }, diff --git a/providers/ofrep/internal/outbound/http.go b/providers/ofrep/internal/outbound/http.go index 630986652..238889f8f 100644 --- a/providers/ofrep/internal/outbound/http.go +++ b/providers/ofrep/internal/outbound/http.go @@ -70,6 +70,7 @@ func (h *Outbound) Single(ctx context.Context, key string, payload []byte) (*Res if err != nil { return nil, err } + defer rsp.Body.Close() b, err := io.ReadAll(rsp.Body) if err != nil { diff --git a/providers/ofrep/internal/outbound/http_test.go b/providers/ofrep/internal/outbound/http_test.go index 0e2947151..80a5aa9e0 100644 --- a/providers/ofrep/internal/outbound/http_test.go +++ b/providers/ofrep/internal/outbound/http_test.go @@ -2,35 +2,17 @@ package outbound import ( "context" - "errors" "fmt" "net/http" + "net/http/httptest" "testing" - "time" ) func TestHttpOutbound(t *testing.T) { // given - host := "localhost:18181" key := "flag" - - server := http.Server{ - Addr: host, - Handler: mockHandler{ - t: t, - key: key, - }, - } - - go func() { - err := server.ListenAndServe() - if err != nil && !errors.Is(err, http.ErrServerClosed) { - t.Logf("error starting mock server: %v", err) - return - } - }() - - <-time.After(3 * time.Second) + server := httptest.NewServer(mockHandler{t: t, key: key}) + t.Cleanup(server.Close) outbound := NewHttp(Configuration{ Callbacks: []HeaderCallback{ @@ -38,7 +20,7 @@ func TestHttpOutbound(t *testing.T) { return "Authorization", "Token" }, }, - BaseURI: fmt.Sprintf("http://%s", host), + BaseURI: server.URL, }) // when @@ -52,12 +34,6 @@ func TestHttpOutbound(t *testing.T) { if response.Status != http.StatusOK { t.Errorf("expected 200, but got %d", response.Status) } - - // cleanup - err = server.Shutdown(context.Background()) - if err != nil { - t.Errorf("error shuttting down mock server: %v", err) - } } type mockHandler struct { diff --git a/providers/ofrep/provider_test.go b/providers/ofrep/provider_test.go index 262871e90..d3ffc4baf 100644 --- a/providers/ofrep/provider_test.go +++ b/providers/ofrep/provider_test.go @@ -2,12 +2,13 @@ package ofrep import ( "context" - "fmt" - "github.com/open-feature/go-sdk/openfeature" "net/http" + "net/http/httptest" "testing" "time" + "github.com/open-feature/go-sdk/openfeature" + "github.com/open-feature/go-sdk-contrib/providers/ofrep/internal/outbound" ) @@ -22,11 +23,11 @@ func TestConfigurations(t *testing.T) { h, v := c.Callbacks[0]() if h != "HEADER" { - t.Errorf(fmt.Sprintf("expected header %s, but got %s", "HEADER", h)) + t.Errorf("expected header %s, but got %s", "HEADER", h) } if v != "VALUE" { - t.Errorf(fmt.Sprintf("expected value %s, but got %s", "VALUE", v)) + t.Errorf("expected value %s, but got %s", "VALUE", v) } }) @@ -38,11 +39,11 @@ func TestConfigurations(t *testing.T) { h, v := c.Callbacks[0]() if h != "Authorization" { - t.Errorf(fmt.Sprintf("expected header %s, but got %s", "Authorization", h)) + t.Errorf("expected header %s, but got %s", "Authorization", h) } if v != "Bearer TOKEN" { - t.Errorf(fmt.Sprintf("expected value %s, but got %s", "Bearer TOKEN", v)) + t.Errorf("expected value %s, but got %s", "Bearer TOKEN", v) } }) @@ -54,44 +55,31 @@ func TestConfigurations(t *testing.T) { h, v := c.Callbacks[0]() if h != "X-API-Key" { - t.Errorf(fmt.Sprintf("expected header %s, but got %s", "X-API-Key", h)) + t.Errorf("expected header %s, but got %s", "X-API-Key", h) } if v != "TOKEN" { - t.Errorf(fmt.Sprintf("expected value %s, but got %s", "TOKEN", v)) + t.Errorf("expected value %s, but got %s", "TOKEN", v) } }) } func TestWiringE2E(t *testing.T) { // mock server with mocked response - host := "localhost:18182" - - server := http.Server{ - Addr: host, - Handler: mockHandler{ + server := httptest.NewServer( + mockHandler{ response: "{\"value\":true,\"key\":\"my-flag\",\"reason\":\"STATIC\",\"variant\":\"true\",\"metadata\":{}}", t: t, }, - } - - go func() { - err := server.ListenAndServe() - if err != nil { - t.Logf("error starting mock server: %v", err) - return - } - }() - - // time for server to be ready - <-time.After(3 * time.Second) + ) + t.Cleanup(server.Close) // custom client with reduced timeout customClient := &http.Client{ Timeout: 1 * time.Second, } - provider := NewProvider(fmt.Sprintf("http://%s", host), WithClient(customClient)) + provider := NewProvider(server.URL, WithClient(customClient)) booleanEvaluation := provider.BooleanEvaluation(context.Background(), "flag", false, nil) if booleanEvaluation.Value != true { @@ -109,11 +97,6 @@ func TestWiringE2E(t *testing.T) { if booleanEvaluation.Error() != nil { t.Errorf("expected no errors, but got %v", booleanEvaluation.Error()) } - - err := server.Shutdown(context.Background()) - if err != nil { - t.Errorf("error shuttting down mock server: %v", err) - } } type mockHandler struct {