Skip to content

fix(ofrep): close http response body after using it #576

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions providers/ofrep/internal/evaluate/flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func TestBooleanEvaluation(t *testing.T) {
expect: false,
},
{
name: "disbaled flag",
name: "disabled flag",
resolver: mockResolver{
success: &successDisabled,
},
Expand Down Expand Up @@ -192,7 +192,7 @@ func TestIntegerEvaluation(t *testing.T) {
expect: 1,
},
{
name: "disbaled flag",
name: "disabled flag",
resolver: mockResolver{
success: &successDisabled,
},
Expand Down Expand Up @@ -250,7 +250,7 @@ func TestFloatEvaluation(t *testing.T) {
expect: 1.05,
},
{
name: "disbaled flag",
name: "disabled flag",
resolver: mockResolver{
success: &successDisabled,
},
Expand Down Expand Up @@ -341,7 +341,7 @@ func TestObjectEvaluation(t *testing.T) {
expect: map[string]interface{}{},
},
{
name: "disbaled flag",
name: "disabled flag",
resolver: mockResolver{
success: &successDisabled,
},
Expand Down
1 change: 1 addition & 0 deletions providers/ofrep/internal/outbound/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
32 changes: 4 additions & 28 deletions providers/ofrep/internal/outbound/http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,43 +2,25 @@ 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{
func() (string, string) {
return "Authorization", "Token"
},
},
BaseURI: fmt.Sprintf("http://%s", host),
BaseURI: server.URL,
})

// when
Expand All @@ -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 {
Expand Down
45 changes: 14 additions & 31 deletions providers/ofrep/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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)
}
})

Expand All @@ -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)
}
})

Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
Loading